[libvirt] [PATCH 03/12] util: json: make value object creator universal by supporting adding

[Michal Privoznik]

On 28.01.2015 11:30, Peter Krempa wrote:
> To allow constructing of value objects stepwise explode the helper into
> separate steps and allow appending into existing value objects.
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/virjson.c       | 74 +++++++++++++++++++++++++++++++-----------------
>  src/util/virjson.h       |  5 ++++
>  3 files changed, 55 insertions(+), 26 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 2bbce03..cc74e35 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1533,6 +1533,8 @@ virJSONValueNewNumberUlong;
>  virJSONValueNewObject;
>  virJSONValueNewString;
>  virJSONValueNewStringLen;
> +virJSONValueObjectAdd;
> +virJSONValueObjectAddVArgs;
>  virJSONValueObjectAppend;
>  virJSONValueObjectAppendBoolean;
>  virJSONValueObjectAppendNull;
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 9f2e1cf..9eb1bff 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -64,12 +64,11 @@ struct _virJSONParser {
> 
> 
>  /**
> - * virJSONValueObjectCreateVArgs:
> - * @obj: returns the created JSON object
> - * @...: a key-value argument pairs, terminated by NULL
> + * virJSONValueObjectAddVArgs:
> + * @obj: JSON object to add the values to
> + * @args: a key-value argument pairs, terminated by NULL
>   *
> - * Creates a JSON value object filled with key-value pairs supplied as variable
> - * argument list.
> + * Adds the key-value pairs supplied as variable argument list to @obj.
>   *
>   * Keys look like   s:name  the first letter is a type code:
>   * Explanation of type codes:
> @@ -104,22 +103,17 @@ struct _virJSONParser {
>   * The value corresponds to the selected type.
>   *
>   * Returns -1 on error. 1 on success, if at least one key:pair was valid 0
> - * in case of no error but nothing was filled (@obj will be NULL).
> + * in case of no error but nothing was filled.
>   */
>  int
> -virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
> +virJSONValueObjectAddVArgs(virJSONValuePtr obj,
> +                           va_list args)
>  {
> -    virJSONValuePtr jargs = NULL;
>      char type;
>      char *key;
>      int ret = -1;
>      int rc;
> 
> -    *obj = NULL;
> -
> -    if (!(jargs = virJSONValueNewObject()))
> -        goto cleanup;
> -
>      while ((key = va_arg(args, char *)) != NULL) {
> 
>          if (strlen(key) < 3) {
> @@ -146,7 +140,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>                                 key);
>                  goto cleanup;
>              }
> -            rc = virJSONValueObjectAppendString(jargs, key, val);
> +            rc = virJSONValueObjectAppendString(obj, key, val);
>          }   break;
> 
>          case 'z':
> @@ -165,7 +159,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>              if (!val && (type == 'z' || type == 'y'))
>                  continue;
> 
> -            rc = virJSONValueObjectAppendNumberInt(jargs, key, val);
> +            rc = virJSONValueObjectAppendNumberInt(obj, key, val);
>          }   break;
> 
>          case 'p':
> @@ -175,7 +169,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>              if (!val && type == 'p')
>                  continue;
> 
> -            rc = virJSONValueObjectAppendNumberUint(jargs, key, val);
> +            rc = virJSONValueObjectAppendNumberUint(obj, key, val);
>          }   break;
> 
>          case 'Z':
> @@ -194,7 +188,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>              if (!val && (type == 'Z' || type == 'Y'))
>                  continue;
> 
> -            rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
> +            rc = virJSONValueObjectAppendNumberLong(obj, key, val);
>          }   break;
> 
>          case 'P':
> @@ -209,12 +203,12 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>              if (!val && type == 'P')
>                  continue;
> 
> -            rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
> +            rc = virJSONValueObjectAppendNumberLong(obj, key, val);
>          }   break;
> 
>          case 'd': {
>              double val = va_arg(args, double);
> -            rc = virJSONValueObjectAppendNumberDouble(jargs, key, val);
> +            rc = virJSONValueObjectAppendNumberDouble(obj, key, val);
>          }   break;
> 
>          case 'B':
> @@ -224,11 +218,11 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>              if (!val && type == 'B')
>                  continue;
> 
> -            rc = virJSONValueObjectAppendBoolean(jargs, key, val);
> +            rc = virJSONValueObjectAppendBoolean(obj, key, val);
>          }   break;
> 
>          case 'n': {
> -            rc = virJSONValueObjectAppendNull(jargs, key);
> +            rc = virJSONValueObjectAppendNull(obj, key);
>          }   break;
> 
>          case 'A':
> @@ -245,7 +239,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>                  goto cleanup;
>              }
> 
> -            rc = virJSONValueObjectAppend(jargs, key, val);
> +            rc = virJSONValueObjectAppend(obj, key, val);
>          }   break;
> 
>          default:
> @@ -259,17 +253,45 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>      }
> 
>      /* verify that we added at least one key-value pair */
> -    if (virJSONValueObjectKeysNumber(jargs) == 0) {
> +    if (virJSONValueObjectKeysNumber(obj) == 0) {
>          ret = 0;
>          goto cleanup;
>      }
> 
> -    *obj = jargs;
> -    jargs = NULL;
>      ret = 1;
> 
>   cleanup:
> -    virJSONValueFree(jargs);
> +    return ret;
> +}
> +
> +
> +int
> +virJSONValueObjectAdd(virJSONValuePtr obj, ...)
> +{
> +    va_list args;
> +    int ret;
> +
> +    va_start(args, obj);
> +    ret = virJSONValueObjectAddVArgs(obj, args);
> +    va_end(args);
> +
> +    return ret;
> +}
> +
> +
> +int
> +virJSONValueObjectCreateVArgs(virJSONValuePtr *obj,
> +                              va_list args)
> +{
> +    int ret;
> +
> +    if (!(*obj = virJSONValueNewObject()))
> +        return -1;
> +
> +    /* free the object on error, or if no value objects were added */
> +    if ((ret = virJSONValueObjectAddVArgs(*obj, args)) <= 0)
> +        VIR_FREE(*obj);
> +

So if I do something like:

virJSONValueObjectCreate(&obj, "s:key1", "val1", "j:key2", -1337, NULL);

the virJSONValueObjectCreateVArgs() will allocate the object, and
virJSONValueObjectAddVArgs() starts adding objects into it. The first
pair will succeed (as long as there's enough memory). The second will,
however, fail, as 'j' format requires nonnegative integer. So we
VIR_FREE() the object constructed so far. Leaving us with a memleak -
the "val1" has been strcpy-ied into obj.

s/VIR_FREE/virJSONValueFree(); *obj = NULL;/

>      return ret;
>  }
> 
> diff --git a/src/util/virjson.h b/src/util/virjson.h
> index c0aefba..fc05ad9 100644
> --- a/src/util/virjson.h
> +++ b/src/util/virjson.h
> @@ -85,6 +85,11 @@ int virJSONValueObjectCreate(virJSONValuePtr *obj, ...)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
>  int virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
>      ATTRIBUTE_NONNULL(1);
> +int virJSONValueObjectAdd(virJSONValuePtr obj, ...)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
> +int virJSONValueObjectAddVArgs(virJSONValuePtr obj, va_list args)
> +    ATTRIBUTE_NONNULL(1);
> +
> 
>  virJSONValuePtr virJSONValueNewString(const char *data);
>  virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length);
> 

Michal