[libvirt] [PATCH 3/5] util: add virTypedParamsPackStrings

[Michal Privoznik]

On 12.05.2015 14:07, Pavel Boldin wrote:
> The `virTypedParamsPackStrings' function provides interface to pack
> multiple string values under the same key to the `virTypedParameter'
> array.
> 
> Signed-off-by: Pavel Boldin <pboldin at mirantis.com>
> ---
>  include/libvirt/libvirt-host.h |  6 +++
>  src/libvirt_public.syms        |  1 +
>  src/util/virtypedparam.c       | 94 +++++++++++++++++++++++++++++++++---------
>  tests/virtypedparamtest.c      | 28 +++++++++++++
>  4 files changed, 109 insertions(+), 20 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
> index afa730f..090ac83 100644
> --- a/include/libvirt/libvirt-host.h
> +++ b/include/libvirt/libvirt-host.h
> @@ -344,6 +344,12 @@ virTypedParamsAddString (virTypedParameterPtr *params,
>                           const char *name,
>                           const char *value);
>  int
> +virTypedParamsPackStrings(virTypedParameterPtr *params,
> +                         int *nparams,
> +                         int *maxparams,
> +                         const char *name,
> +                         const char **values);

Rather unusual name. How about virTypedParamsAddStringList()?

> +int
>  virTypedParamsAddFromString(virTypedParameterPtr *params,
>                           int *nparams,
>                           int *maxparams,
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index d886967..8a24bb6 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -714,6 +714,7 @@ LIBVIRT_1.2.16 {
>      global:
>          virTypedParamsPick;
>          virTypedParamsPickStrings;
> +        virTypedParamsPackStrings;
>  } LIBVIRT_1.2.15;
>  
>  # .... define new API here using predicted next version number ....
> diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
> index 643d10e..9f2ab3c 100644
> --- a/src/util/virtypedparam.c
> +++ b/src/util/virtypedparam.c
> @@ -1132,6 +1132,43 @@ virTypedParamsAddBoolean(virTypedParameterPtr *params,
>      return -1;
>  }
>  
> +static int
> +virTypedParamsAddStringFull(virTypedParameterPtr *params,
> +                            int *nparams,
> +                            int *maxparams,
> +                            const char *name,
> +                            const char *value,
> +                            bool unique)
> +{
> +    char *str = NULL;
> +    size_t max = *maxparams;
> +    size_t n = *nparams;
> +
> +    virResetLastError();

No. This is not a public API. This has nothing to do here.

> +
> +    if (unique)
> +        VIR_TYPED_PARAM_CHECK();

So you are kind of trying to solve the problem I've raised in 1/5.
Unfortunately, I guess we ought to drop the check entirely. Not only for
strings, but for other types too.

> +    if (VIR_RESIZE_N(*params, max, n, 1) < 0)
> +        goto error;
> +    *maxparams = max;
> +
> +    if (VIR_STRDUP(str, value) < 0)
> +        goto error;
> +
> +    if (virTypedParameterAssign(*params + n, name,
> +                                VIR_TYPED_PARAM_STRING, str) < 0) {
> +        VIR_FREE(str);
> +        goto error;
> +    }
> +
> +    *nparams += 1;
> +    return 0;
> +
> + error:
> +    virDispatchError(NULL);
> +    return -1;
> +}
> +
>  
>  /**
>   * virTypedParamsAddString:
> @@ -1160,32 +1197,49 @@ virTypedParamsAddString(virTypedParameterPtr *params,
>                          const char *name,
>                          const char *value)
>  {
> -    char *str = NULL;
> -    size_t max = *maxparams;
> -    size_t n = *nparams;
> +    return virTypedParamsAddStringFull(params,
> +                                       nparams,
> +                                       maxparams,
> +                                       name,
> +                                       value,
> +                                       1);

s/1/true/

> +}
>  
> -    virResetLastError();
>  
> -    VIR_TYPED_PARAM_CHECK();
> -    if (VIR_RESIZE_N(*params, max, n, 1) < 0)
> -        goto error;
> -    *maxparams = max;
> +/**
> + * virTypedParamsPackStrings:
> + * @params: array of typed parameters
> + * @nparams: number of parameters in the @params array
> + * @maxparams: maximum number of parameters that can be stored in @params
> + *      array without allocating more memory
> + * @name: name of the parameter to store values to
> + * @values: the values to store into the new parameters
> + *
> + * Packs NULL-terminated list of strings @values into @params under the
> + * key @name.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int
> +virTypedParamsPackStrings(virTypedParameterPtr *params,
> +                          int *nparams,
> +                          int *maxparams,
> +                          const char *name,
> +                          const char **values)
> +{
> +    size_t i;
> +    int rv = -1;
>  
> -    if (VIR_STRDUP(str, value) < 0)
> -        goto error;
> +    if (!values)
> +        return 0;
>  
> -    if (virTypedParameterAssign(*params + n, name,
> -                                VIR_TYPED_PARAM_STRING, str) < 0) {
> -        VIR_FREE(str);
> -        goto error;
> +    for (i = 0; values[i]; i++) {
> +        if ((rv = virTypedParamsAddStringFull(params, nparams, maxparams,
> +                                              name, values[i], 0)) < 0)

s/0/false/

> +            break;
>      }
>  
> -    *nparams += 1;
> -    return 0;
> -
> - error:
> -    virDispatchError(NULL);
> -    return -1;
> +    return rv;
>  }
>  
>  
> diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c
> index 287d3f1..037d5d1 100644
> --- a/tests/virtypedparamtest.c
> +++ b/tests/virtypedparamtest.c
> @@ -139,6 +139,31 @@ testTypedParamsPick(const void *opaque ATTRIBUTE_UNUSED)
>  }
>  
>  static int
> +testTypedParamsPackStrings(const void *opaque ATTRIBUTE_UNUSED)
> +{
> +    int rv = 0;
> +    virTypedParameterPtr params = NULL;
> +    int nparams = 0, maxparams = 0, i;
> +
> +    const char *values[] = {
> +        "foo", "bar", "foobar", NULL
> +    };
> +
> +    rv = virTypedParamsPackStrings(&params, &nparams, &maxparams, "param",
> +                                   values);
> +
> +    for (i = 0; i < nparams; i++) {
> +        if (STRNEQ(params[i].field, "param") ||
> +            STRNEQ(params[i].value.s, values[i]) ||
> +            params[i].type != VIR_TYPED_PARAM_STRING)
> +            rv = -1;
> +    }
> +
> +    virTypedParamsFree(params, nparams);
> +    return rv;
> +}
> +
> +static int
>  testTypedParamsPickStrings(const void *opaque ATTRIBUTE_UNUSED)
>  {
>      size_t i;
> @@ -256,6 +281,9 @@ mymain(void)
>      if (virtTestRun("Picking Strings", testTypedParamsPickStrings, NULL) < 0)
>          rv = -1;
>  
> +    if (virtTestRun("Packing Strings", testTypedParamsPackStrings, NULL) < 0)
> +        rv = -1;
> +
>      if (rv < 0)
>          return EXIT_FAILURE;
>      return EXIT_SUCCESS;
> 

Michal