Eric Blake <ebl...@redhat.com> 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 <arm...@redhat.com> >> --- >> 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, &s); >> + >> + 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(&children); >> + object_child_foreach(obj, insert_qom_composition_child, &children); >> + while ((child = g_queue_pop_head(&children))) { >> + 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!