Re: [PATCH 04/21] Introduce virTypedParamsReplaceString internal API

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 18.06.2013 16:05, Jiri Denemark wrote:
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virtypedparam.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virtypedparam.h |  5 +++++
>  3 files changed, 64 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ae9f356..4e01073 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1900,6 +1900,7 @@ virTPMCreateCancelPath;
>  virTypedParameterAssign;
>  virTypedParameterAssignFromStr;
>  virTypedParamsCheck;
> +virTypedParamsReplaceString;
>  virTypedParamsValidate;
>  
>  
> diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
> index 825148b..760bb05 100644
> --- a/src/util/virtypedparam.c
> +++ b/src/util/virtypedparam.c
> @@ -285,6 +285,64 @@ cleanup:
>  }
>  
>  
> +/**
> + * virTypedParamsReplaceString:
> + * @params: pointer to the array of typed parameters
> + * @nparams: number of parameters in the @params array
> + * @name: name of the parameter to set
> + * @value: the value to store into the parameter
> + *
> + * Sets new value @value to parameter called @name with char * type. If the
> + * parameter does not exist yet in @params, it is automatically created and
> + * @naprams is incremented by one. Otherwise current value of the parameter
> + * is freed on success. The function creates its own copy of @value string,
> + * which needs to be freed using virTypedParamsFree or virTypedParamsClear.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int
> +virTypedParamsReplaceString(virTypedParameterPtr *params,
> +                            int *nparams,
> +                            const char *name,
> +                            const char *value)
> +{
> +    char *str = NULL;
> +    char *old = NULL;
> +    size_t n = *nparams;
> +    virTypedParameterPtr param;
> +
> +    virResetLastError();
> +
> +    param = virTypedParamsGet(*params, n, name);

You need to check if the param is a type of VIR_TYPED_PARAM_STRING.

> +    if (param) {
> +        old = param->value.s;
> +    } else if (VIR_EXPAND_N(*params, n, 1) < 0) {
> +        virReportOOMError();
> +        goto error;
> +    } else {
> +        param = *params + n - 1;
> +    }

Please no. I'd rather see this as:
     if (param) {
        old = param->value.s;
     } else {
        if (VIR_EXPAND_N(*params, n, 1) < 0) {
            virReportOOMError();
            goto error;
        }
        param = *params + n - 1;
     }

> +
> +    if (VIR_STRDUP(str, value) < 0)
> +        goto error;
> +
> +    if (virTypedParameterAssign(param, name,
> +                                VIR_TYPED_PARAM_STRING, str) < 0) {
> +        param->value.s = old;
> +        VIR_FREE(str);
> +        goto error;
> +    }
> +    VIR_FREE(old);
> +
> +    *nparams = n;
> +    return 0;
> +
> +error:
> +    virDispatchError(NULL);
> +    return -1;
> +}
> +
> +
>  /* The following APIs are public and their signature may never change. */
>  
>  /**
> diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h
> index b0f8522..6eb61c4 100644
> --- a/src/util/virtypedparam.h
> +++ b/src/util/virtypedparam.h
> @@ -44,4 +44,9 @@ int virTypedParameterAssignFromStr(virTypedParameterPtr param,
>                                     const char *val)
>      ATTRIBUTE_RETURN_CHECK;
>  
> +int virTypedParamsReplaceString(virTypedParameterPtr *params,
> +                                int *nparams,
> +                                const char *name,
> +                                const char *value);
> +
>  #endif /* __VIR_TYPED_PARAM_H */
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list