[libvirt] [PATCH 05/21] Introduce VIR_TYPED_PARAMS_DEBUG macro for dumping typed params

[Michal Privoznik]

On 18.06.2013 16:05, Jiri Denemark wrote:
> All APIs that take typed parameters are only using params address in
> their entry point debug messages. With the new VIR_TYPED_PARAMS_DEBUG
> macro, all functions can easily log all individual typed parameters
> passed to them.
> ---
>  docs/apibuild.py         |  1 +
>  src/libvirt_private.syms |  3 +++
>  src/util/virtypedparam.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  src/util/virtypedparam.h | 18 ++++++++++++++++++
>  4 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/apibuild.py b/docs/apibuild.py
> index c816197..e0996bf 100755
> --- a/docs/apibuild.py
> +++ b/docs/apibuild.py
> @@ -67,6 +67,7 @@ ignored_functions = {
>    "virTypedParamsValidate": "internal function in virtypedparam.c",
>    "virTypedParameterAssign": "internal function in virtypedparam.c",
>    "virTypedParameterAssignFromStr": "internal function in virtypedparam.c",
> +  "virTypedParameterToString": "internal function in virtypedparam.c",
>    "virTypedParamsCheck": "internal function in virtypedparam.c",
>  }
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 4e01073..0e0c3bc 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1899,6 +1899,9 @@ virTPMCreateCancelPath;
>  # util/virtypedparam.h
>  virTypedParameterAssign;
>  virTypedParameterAssignFromStr;
> +virTypedParameterToString;
> +virTypedParameterTypeFromString;
> +virTypedParameterTypeToString;
>  virTypedParamsCheck;
>  virTypedParamsReplaceString;
>  virTypedParamsValidate;
> diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
> index 760bb05..8b18a5a 100644
> --- a/src/util/virtypedparam.c
> +++ b/src/util/virtypedparam.c
> @@ -31,7 +31,6 @@
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
> -VIR_ENUM_DECL(virTypedParameter)
>  VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST,
>                "unknown",
>                "int",
> @@ -135,6 +134,46 @@ virTypedParamsCheck(virTypedParameterPtr params,
>      return true;
>  }
>  
> +char *
> +virTypedParameterToString(virTypedParameterPtr param)
> +{
> +    char *value;
> +    int ret = -1;
> +
> +    switch (param->type) {
> +    case VIR_TYPED_PARAM_INT:
> +        ret = virAsprintf(&value, "%d", param->value.i);
> +        break;
> +    case VIR_TYPED_PARAM_UINT:
> +        ret = virAsprintf(&value, "%u", param->value.ui);
> +        break;
> +    case VIR_TYPED_PARAM_LLONG:
> +        ret = virAsprintf(&value, "%lld", param->value.l);
> +        break;
> +    case VIR_TYPED_PARAM_ULLONG:
> +        ret = virAsprintf(&value, "%llu", param->value.ul);
> +        break;
> +    case VIR_TYPED_PARAM_DOUBLE:
> +        ret = virAsprintf(&value, "%g", param->value.d);
> +        break;
> +    case VIR_TYPED_PARAM_BOOLEAN:
> +        ret = virAsprintf(&value, "%d", param->value.b);

virAsprintf() doesn't report OOM error (yet) ...

> +        break;
> +    case VIR_TYPED_PARAM_STRING:
> +        ret = VIR_STRDUP(value, param->value.s);

... while VIR_STRDUP does.

> +        break;
> +    default:
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("unexpected type %d for field %s"),
> +                       param->type, param->field);
> +    }
> +
> +    if (ret < 0)
> +        return NULL;

But I guess it doesn't matter, does it (esp. when each call is followed
by virResetLastError();)

> +    else
> +        return value;
> +}
> +
>  /* Assign name, type, and the appropriately typed arg to param; in the
>   * case of a string, the caller is assumed to have malloc'd a string,
>   * or can pass NULL to have this function malloc an empty string.
> diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h
> index 6eb61c4..ca062c0 100644
> --- a/src/util/virtypedparam.h
> +++ b/src/util/virtypedparam.h
> @@ -24,6 +24,7 @@
>  # define __VIR_TYPED_PARAM_H_
>  
>  # include "internal.h"
> +# include "virutil.h"
>  
>  int virTypedParamsValidate(virTypedParameterPtr params, int nparams,
>                             /* const char *name, int type ... */ ...)
> @@ -49,4 +50,21 @@ int virTypedParamsReplaceString(virTypedParameterPtr *params,
>                                  const char *name,
>                                  const char *value);
>  
> +char *virTypedParameterToString(virTypedParameterPtr param);
> +
> +VIR_ENUM_DECL(virTypedParameter)
> +
> +# define VIR_TYPED_PARAMS_DEBUG(params, nparams)                            \
> +    do {                                                                    \

I'd rather check (params) != NULL here as it might avoid SIGSEGV in the
next patch if user calls i.e. virDomainSetSchedulerParametersFlags(dom,
NULL, 10, 0); But that's not a show stopper as users might pass invalid
pointer anyway.

> +        int _i;                                                             \
> +        for (_i = 0; _i < (nparams); _i++) {                                \
> +            char *_value = virTypedParameterToString((params) + _i);        \
> +            VIR_DEBUG("params[\"%s\"]=(%s)%s",                              \
> +                      (params)[_i].field,                                   \
> +                      virTypedParameterTypeToString((params)[_i].type),     \
> +                      NULLSTR(_value));                                     \
> +            VIR_FREE(_value);                                               \
> +        }                                                                   \
> +    } while (0)
> +
>  #endif /* __VIR_TYPED_PARAM_H */
>