Re: [PATCH V4 3/5] util: strv_from_strList

[Markus Armbruster <armbru@redhat.com>]

Steve Sistare <steven.sistare@oracle.com> writes:

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/strList.h |  6 ++++++
>  util/strList.c         | 14 ++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/include/qemu/strList.h b/include/qemu/strList.h
> index 010237f..4b86aa6 100644
> --- a/include/qemu/strList.h
> +++ b/include/qemu/strList.h
> @@ -21,4 +21,10 @@
>   */
>  strList *strList_from_string(const char *in, const char *delim);
>  
> +/*
> + * Produce and return a NULL-terminated array of strings from @args.
> + * The result is g_malloc'd and all strings are g_strdup'd.
> + */
> +GStrv strv_from_strList(const strList *args);
> +
>  #endif
> diff --git a/util/strList.c b/util/strList.c
> index 7991de3..bad4187 100644
> --- a/util/strList.c
> +++ b/util/strList.c
> @@ -22,3 +22,17 @@ strList *strList_from_string(const char *str, const char *delim)
>  
>      return res;
>  }
> +
> +GStrv strv_from_strList(const strList *args)

Suggest to name the argument @list.

> +{
> +    const strList *arg;

Suggest to name this @tail.

> +    int i = 0;
> +    GStrv argv = g_new(char *, QAPI_LIST_LENGTH(args) + 1);
> +
> +    for (arg = args; arg != NULL; arg = arg->next) {
> +        argv[i++] = g_strdup(arg->value);
> +    }
> +    argv[i] = NULL;
> +
> +    return argv;
> +}

Can we use char ** instread of GStrv?  I'd find that clearer.  For what
it's worth, GLib documentation of functions like g_strsplit() doesn't
use the GStrv typedef, either.