Re: [PATCH not-for-merge 2/5] qom: Make "info qom-tree" show children sorted
Eric Blake writes: > On 5/18/20 12:19 AM, Markus Armbruster wrote: >> "info qom-tree" prints children in unstable order. This is a pain >> when diffing output for different versions to find change. Print it >> sorted. > > Yes, this does seem reasonable to include even without the rest of the > series. Noted. >> Signed-off-by: Markus Armbruster >> --- >> qom/qom-hmp-cmds.c | 40 +++- >> 1 file changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c >> index 4a61ee1b8c..cf0af8f6b5 100644 >> --- a/qom/qom-hmp-cmds.c >> +++ b/qom/qom-hmp-cmds.c >> @@ -78,6 +78,35 @@ static int print_qom_composition_child(Object *obj, void >> *opaque) >> return 0; >> } >> +static int qom_composition_compare(const void *a, const void *b, >> void *ignore) >> +{ >> +Object *obja = (void *)a, *objb = (void *)b; > > Casting away const... > >> +const char *namea, *nameb; >> + >> +if (obja == object_get_root()) { >> +namea = g_strdup(""); >> +} else { >> +namea = object_get_canonical_path_component(obja); > > ...should we instead improve object_get_canonical_path_component to > work with 'const Object *'? Go right ahead :) I need to sit on my hands to have a chance getting my task queue back under control. >> +} >> + >> +if (objb == object_get_root()) { >> +nameb = g_strdup(""); >> +} else { >> +nameb = object_get_canonical_path_component(objb); >> +} >> + >> + >> +return strcmp(namea, nameb); > > Why the two blank lines? This leaks namea and/or nameb if either > object is the object root. Should you instead use g_strcmp0 here, > with namea/b set to NULL instead of g_strdup("") above? My not-for-merge proves prudent ;) >> @@ -105,7 +134,16 @@ static void print_qom_composition(Monitor *mon, Object >> *obj, int indent) >> monitor_printf(mon, "%*s/%s (%s)\n", indent, "", name, >> object_get_typename(obj)); >> g_free(name); >> -object_child_foreach(obj, print_qom_composition_child, ); >> + >> +GQueue children; >> +Object *child; > > Mid-function declarations - I assume you'd clean this up if we want > this for real? Yes. I prioritized diff over maintainability, because not-for-merge. >> +g_queue_init(); >> +object_child_foreach(obj, insert_qom_composition_child, ); >> +while ((child = g_queue_pop_head())) { >> +print_qom_composition(mon, child, indent + 2); >> +} >> +(void)s; >> +(void)print_qom_composition_child; > > Also, this looks like leftover debugger aids? Shut up the compiler so I don't have to remove code. Shorter diff, not-for-merge. >> } >> void hmp_info_qom_tree(Monitor *mon, const QDict *dict) >> Thanks!
Re: [PATCH not-for-merge 2/5] qom: Make "info qom-tree" show children sorted
On 5/18/20 12:19 AM, Markus Armbruster wrote: "info qom-tree" prints children in unstable order. This is a pain when diffing output for different versions to find change. Print it sorted. Yes, this does seem reasonable to include even without the rest of the series. Signed-off-by: Markus Armbruster --- qom/qom-hmp-cmds.c | 40 +++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c index 4a61ee1b8c..cf0af8f6b5 100644 --- a/qom/qom-hmp-cmds.c +++ b/qom/qom-hmp-cmds.c @@ -78,6 +78,35 @@ static int print_qom_composition_child(Object *obj, void *opaque) return 0; } +static int qom_composition_compare(const void *a, const void *b, void *ignore) +{ +Object *obja = (void *)a, *objb = (void *)b; Casting away const... +const char *namea, *nameb; + +if (obja == object_get_root()) { +namea = g_strdup(""); +} else { +namea = object_get_canonical_path_component(obja); ...should we instead improve object_get_canonical_path_component to work with 'const Object *'? +} + +if (objb == object_get_root()) { +nameb = g_strdup(""); +} else { +nameb = object_get_canonical_path_component(objb); +} + + +return strcmp(namea, nameb); Why the two blank lines? This leaks namea and/or nameb if either object is the object root. Should you instead use g_strcmp0 here, with namea/b set to NULL instead of g_strdup("") above? @@ -105,7 +134,16 @@ static void print_qom_composition(Monitor *mon, Object *obj, int indent) monitor_printf(mon, "%*s/%s (%s)\n", indent, "", name, object_get_typename(obj)); g_free(name); -object_child_foreach(obj, print_qom_composition_child, ); + +GQueue children; +Object *child; Mid-function declarations - I assume you'd clean this up if we want this for real? +g_queue_init(); +object_child_foreach(obj, insert_qom_composition_child, ); +while ((child = g_queue_pop_head())) { +print_qom_composition(mon, child, indent + 2); +} +(void)s; +(void)print_qom_composition_child; Also, this looks like leftover debugger aids? } void hmp_info_qom_tree(Monitor *mon, const QDict *dict) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH not-for-merge 2/5] qom: Make "info qom-tree" show children sorted
"info qom-tree" prints children in unstable order. This is a pain when diffing output for different versions to find change. Print it sorted. Signed-off-by: Markus Armbruster --- qom/qom-hmp-cmds.c | 40 +++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c index 4a61ee1b8c..cf0af8f6b5 100644 --- a/qom/qom-hmp-cmds.c +++ b/qom/qom-hmp-cmds.c @@ -78,6 +78,35 @@ static int print_qom_composition_child(Object *obj, void *opaque) return 0; } +static int qom_composition_compare(const void *a, const void *b, void *ignore) +{ +Object *obja = (void *)a, *objb = (void *)b; +const char *namea, *nameb; + +if (obja == object_get_root()) { +namea = g_strdup(""); +} else { +namea = object_get_canonical_path_component(obja); +} + +if (objb == object_get_root()) { +nameb = g_strdup(""); +} else { +nameb = object_get_canonical_path_component(objb); +} + + +return strcmp(namea, nameb); +} + +static int insert_qom_composition_child(Object *obj, void *opaque) +{ +GQueue *children = opaque; + + g_queue_insert_sorted(children, obj, qom_composition_compare, NULL); + return 0; +} + static void print_qom_composition(Monitor *mon, Object *obj, int indent) { QOMCompositionState s = { @@ -105,7 +134,16 @@ static void print_qom_composition(Monitor *mon, Object *obj, int indent) monitor_printf(mon, "%*s/%s (%s)\n", indent, "", name, object_get_typename(obj)); g_free(name); -object_child_foreach(obj, print_qom_composition_child, ); + +GQueue children; +Object *child; +g_queue_init(); +object_child_foreach(obj, insert_qom_composition_child, ); +while ((child = g_queue_pop_head())) { +print_qom_composition(mon, child, indent + 2); +} +(void)s; +(void)print_qom_composition_child; } void hmp_info_qom_tree(Monitor *mon, const QDict *dict) -- 2.21.1