Re: [PATCH not-for-merge 2/5] qom: Make "info qom-tree" show children sorted

[Eric Blake]

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 <address@hidden>
> ---
>   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, &s);
> +
> +    GQueue children;
> +    Object *child;

Mid-function declarations - I assume you'd clean this up if we want this 
for real?

> +    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?

>   }
>   
>   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