Re: [PATCH v2 2/4] qom: Factor out helpers from user_creatable_print_help()

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> This creates separate helper functions for printing a list of user
> creatable object types and for printing a list of properties of a given
> type. This allows using these parts without having a QemuOpts.

Does the last sentence allude to a future patch?  If yes, I suggest to
phrase it as "This will allow ..."

> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qom/object_interfaces.c | 90 ++++++++++++++++++++++++-----------------
>  1 file changed, 52 insertions(+), 38 deletions(-)
>
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index e8e1523960..3fd1da157e 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -214,54 +214,68 @@ char *object_property_help(const char *name, const char 
> *type,
>      return g_string_free(str, false);
>  }
>  
> -bool user_creatable_print_help(const char *type, QemuOpts *opts)
> +static void user_creatable_print_types(void)
> +{
> +    GSList *l, *list;
> +
> +    printf("List of user creatable objects:\n");
> +    list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
> +    for (l = list; l != NULL; l = l->next) {
> +        ObjectClass *oc = OBJECT_CLASS(l->data);
> +        printf("  %s\n", object_class_get_name(oc));
> +    }
> +    g_slist_free(list);
> +}
> +
> +static bool user_creatable_print_type_properites(const char *type)
>  {
>      ObjectClass *klass;
> +    ObjectPropertyIterator iter;
> +    ObjectProperty *prop;
> +    GPtrArray *array;
> +    int i;
>  
> -    if (is_help_option(type)) {
> -        GSList *l, *list;
> +    klass = object_class_by_name(type);
> +    if (!klass) {
> +        return false;
> +    }
>  
> -        printf("List of user creatable objects:\n");
> -        list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
> -        for (l = list; l != NULL; l = l->next) {
> -            ObjectClass *oc = OBJECT_CLASS(l->data);
> -            printf("  %s\n", object_class_get_name(oc));
> +    array = g_ptr_array_new();
> +    object_class_property_iter_init(&iter, klass);
> +    while ((prop = object_property_iter_next(&iter))) {
> +        if (!prop->set) {
> +            continue;
>          }
> -        g_slist_free(list);
> -        return true;
> +
> +        g_ptr_array_add(array,
> +                        object_property_help(prop->name, prop->type,
> +                                             prop->defval, 
> prop->description));
>      }
> +    g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
> +    if (array->len > 0) {
> +        printf("%s options:\n", type);
> +    } else {
> +        printf("There are no options for %s.\n", type);
> +    }
> +    for (i = 0; i < array->len; i++) {
> +        printf("%s\n", (char *)array->pdata[i]);
> +    }
> +    g_ptr_array_set_free_func(array, g_free);
> +    g_ptr_array_free(array, true);
> +    return true;
> +}
>  
> -    klass = object_class_by_name(type);
> -    if (klass && qemu_opt_has_help_opt(opts)) {
> -        ObjectPropertyIterator iter;
> -        ObjectProperty *prop;
> -        GPtrArray *array = g_ptr_array_new();
> -        int i;
> -
> -        object_class_property_iter_init(&iter, klass);
> -        while ((prop = object_property_iter_next(&iter))) {
> -            if (!prop->set) {
> -                continue;
> -            }
> -
> -            g_ptr_array_add(array,
> -                            object_property_help(prop->name, prop->type,
> -                                                 prop->defval, 
> prop->description));
> -        }
> -        g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
> -        if (array->len > 0) {
> -            printf("%s options:\n", type);
> -        } else {
> -            printf("There are no options for %s.\n", type);
> -        }
> -        for (i = 0; i < array->len; i++) {
> -            printf("%s\n", (char *)array->pdata[i]);
> -        }
> -        g_ptr_array_set_free_func(array, g_free);
> -        g_ptr_array_free(array, true);
> +bool user_creatable_print_help(const char *type, QemuOpts *opts)
> +{
> +    if (is_help_option(type)) {
> +        user_creatable_print_types();
>          return true;
>      }
>  
> +    if (qemu_opt_has_help_opt(opts)) {
> +        return user_creatable_print_type_properites(type);
> +    }
> +
>      return false;
>  }

I'd make user_creatable_print_types() return true for summetry with
user_creatable_print_type_properites(), but that's a matter of taste.

Reviewed-by: Markus Armbruster <armbru@redhat.com>