On 10/14/2014 03:29 AM, Peter Krempa wrote:
> Our qemu monitor code has a converter from key-value pairs to a json
> value object. I want to re-use the code later and having it part of the
> monitor command generator is inflexible. Split it out into a separate
> helper.
> ---
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_monitor_json.c | 161 +--------------------------------
> src/util/virjson.c | 211 +++++++++++++++++++++++++++++++++++++++++++
> src/util/virjson.h | 5 +
> 4 files changed, 220 insertions(+), 159 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d6265ac..314b2b8 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1503,6 +1503,8 @@ virJSONValueObjectAppendNumberLong;
> virJSONValueObjectAppendNumberUint;
> virJSONValueObjectAppendNumberUlong;
> virJSONValueObjectAppendString;
> +virJSONValueObjectCreate;
> +virJSONValueObjectCreateVArgs;
> virJSONValueObjectGet;
> virJSONValueObjectGetBoolean;
> virJSONValueObjectGetKey;
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 7f23aae..2967193 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -431,7 +431,6 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char *cmdname, ...)
> virJSONValuePtr obj;
> virJSONValuePtr jargs = NULL;
> va_list args;
> - char *key;
>
> va_start(args, cmdname);
>
> @@ -442,164 +441,8 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char *cmdname, ...)
> cmdname) < 0)
> goto error;
>
> - while ((key = va_arg(args, char *)) != NULL) {
> - int ret;
> - char type;
> -
> - if (strlen(key) < 3) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("argument key '%s' is too short, missing type prefix"),
> - key);
> - goto error;
> - }
> -
> - /* Keys look like s:name the first letter is a type code:
> - * Explanation of type codes:
> - * s: string value, must be non-null
> - * S: string value, omitted if null
> - *
> - * i: signed integer value
> - * j: signed integer value, error if negative
> - * z: signed integer value, omitted if zero
> - * y: signed integer value, omitted if zero, error if negative
> - *
> - * I: signed long integer value
> - * J: signed long integer value, error if negative
> - * Z: signed long integer value, omitted if zero
> - * Y: signed long integer value, omitted if zero, error if negative
> - *
> - * u: unsigned integer value
> - * p: unsigned integer value, omitted if zero
> - *
> - * U: unsigned long integer value (see below for quirks)
> - * P: unsigned long integer value, omitted if zero
> - *
> - * b: boolean value
> - * B: boolean value, omitted if false
> - *
> - * d: double precision floating point number
> - * n: json null value
> - * a: json array
> - */
> - type = key[0];
> - key += 2;
> -
> - if (!jargs &&
> - !(jargs = virJSONValueNewObject()))
> - goto error;
> -
> - /* This doesn't support maps, but no command uses those. */
> - switch (type) {
> - case 'S':
> - case 's': {
> - char *val = va_arg(args, char *);
> - if (!val) {
> - if (type == 'S')
> - continue;
> -
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("argument key '%s' must not have null value"),
> - key);
> - goto error;
> - }
> - ret = virJSONValueObjectAppendString(jargs, key, val);
> - } break;
> -
> - case 'z':
> - case 'y':
> - case 'j':
> - case 'i': {
> - int val = va_arg(args, int);
> -
> - if (val < 0 && (type == 'j' || type == 'y')) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("argument key '%s' must not be negative"),
> - key);
> - goto error;
> - }
> -
> - if (!val && (type == 'z' || type == 'y'))
> - continue;
> -
> - ret = virJSONValueObjectAppendNumberInt(jargs, key, val);
> - } break;
> -
> - case 'p':
> - case 'u': {
> - unsigned int val = va_arg(args, unsigned int);
> -
> - if (!val && type == 'p')
> - continue;
> -
> - ret = virJSONValueObjectAppendNumberUint(jargs, key, val);
> - } break;
> -
> - case 'Z':
> - case 'Y':
> - case 'J':
> - case 'I': {
> - long long val = va_arg(args, long long);
> -
> - if (val < 0 && (type == 'J' || type == 'Y')) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("argument key '%s' must not be negative"),
> - key);
> - goto error;
> - }
> -
> - if (!val && (type == 'Z' || type == 'Y'))
> - continue;
> -
> - ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
> - } break;
> -
> - case 'P':
> - case 'U': {
> - /* qemu silently truncates numbers larger than LLONG_MAX,
> - * so passing the full range of unsigned 64 bit integers
> - * is not safe here. Pass them as signed 64 bit integers
> - * instead.
> - */
> - long long val = va_arg(args, long long);
> -
> - if (!val && type == 'P')
> - continue;
> -
> - ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
> - } break;
> -
> - case 'd': {
> - double val = va_arg(args, double);
> - ret = virJSONValueObjectAppendNumberDouble(jargs, key, val);
> - } break;
> -
> - case 'B':
> - case 'b': {
> - int val = va_arg(args, int);
> -
> - if (!val && type == 'B')
> - continue;
> -
> - ret = virJSONValueObjectAppendBoolean(jargs, key, val);
> - } break;
> -
> - case 'n': {
> - ret = virJSONValueObjectAppendNull(jargs, key);
> - } break;
> -
> - case 'a': {
> - virJSONValuePtr val = va_arg(args, virJSONValuePtr);
> - ret = virJSONValueObjectAppend(jargs, key, val);
> - } break;
> -
> - default:
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("unsupported data type '%c' for arg '%s'"), type, key - 2);
> - goto error;
> - }
> - if (ret < 0)
> - goto error;
> - }
> + if (virJSONValueObjectCreateVArgs(&jargs, args) < 0)
> + goto error;
>
> if (jargs &&
> virJSONValueObjectAppend(obj, wrap ? "data" : "arguments", jargs) < 0)
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index ec83b2f..0dfeaed 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -63,6 +63,217 @@ struct _virJSONParser {
> };
>
>
> +/**
> + * virJSONValueObjectCreateVArgs:
> + * @obj: returns the created JSON object
> + * @...: a key-value argument pairs, terminated by NULL
> + *
> + * Creates a JSON value object filled with key-value pairs supplied as variable
> + * argument list.
> + *
> + * Keys look like s:name the first letter is a type code:
> + * Explanation of type codes:
> + * s: string value, must be non-null
> + * S: string value, omitted if null
> + *
> + * i: signed integer value
> + * j: signed integer value, error if negative
> + * z: signed integer value, omitted if zero
> + * y: signed integer value, omitted if zero, error if negative
> + *
> + * I: signed long integer value
> + * J: signed long integer value, error if negative
> + * Z: signed long integer value, omitted if zero
> + * Y: signed long integer value, omitted if zero, error if negative
> + *
> + * u: unsigned integer value
> + * p: unsigned integer value, omitted if zero
> + *
> + * U: unsigned long integer value (see below for quirks)
> + * P: unsigned long integer value, omitted if zero
> + *
> + * b: boolean value
> + * B: boolean value, omitted if false
> + *
> + * d: double precision floating point number
> + * n: json null value
> + * a: json array
> + *
> + * 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).
> + */
> +int
> +virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
> +{
> + virJSONValuePtr jargs = NULL;
> + char type;
> + char *key;
> + int ret = -1;
> + int rc = -2; /* -2 is a sentinel to check if at least one entry was added */
> +
> + *obj = NULL;
> +
> + if (!(jargs = virJSONValueNewObject()))
> + goto cleanup;
> +
> + while ((key = va_arg(args, char *)) != NULL) {
> +
> + if (strlen(key) < 3) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("argument key '%s' is too short, missing type prefix"),
> + key);
> + goto cleanup;
> + }
> +
> + type = key[0];
> + key += 2;
> +
> + /* This doesn't support maps, but no command uses those. */
> + switch (type) {
> + case 'S':
> + case 's': {
> + char *val = va_arg(args, char *);
> + if (!val) {
> + if (type == 'S')
> + continue;
> +
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("argument key '%s' must not have null value"),
> + key);
> + goto cleanup;
> + }
> + rc = virJSONValueObjectAppendString(jargs, key, val);
> + } break;
> +
> + case 'z':
> + case 'y':
> + case 'j':
> + case 'i': {
> + int val = va_arg(args, int);
> +
> + if (val < 0 && (type == 'j' || type == 'y')) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("argument key '%s' must not be negative"),
> + key);
> + goto cleanup;
> + }
> +
> + if (!val && (type == 'z' || type == 'y'))
> + continue;
> +
> + rc = virJSONValueObjectAppendNumberInt(jargs, key, val);
> + } break;
> +
> + case 'p':
> + case 'u': {
> + unsigned int val = va_arg(args, unsigned int);
> +
> + if (!val && type == 'p')
> + continue;
> +
> + rc = virJSONValueObjectAppendNumberUint(jargs, key, val);
> + } break;
> +
> + case 'Z':
> + case 'Y':
> + case 'J':
> + case 'I': {
> + long long val = va_arg(args, long long);
> +
> + if (val < 0 && (type == 'J' || type == 'Y')) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("argument key '%s' must not be negative"),
> + key);
> + goto cleanup;
> + }
> +
> + if (!val && (type == 'Z' || type == 'Y'))
> + continue;
> +
> + rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
> + } break;
> +
> + case 'P':
> + case 'U': {
> + /* qemu silently truncates numbers larger than LLONG_MAX,
> + * so passing the full range of unsigned 64 bit integers
> + * is not safe here. Pass them as signed 64 bit integers
> + * instead.
> + */
> + long long val = va_arg(args, long long);
> +
> + if (!val && type == 'P')
> + continue;
> +
> + rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
> + } break;
> +
> + case 'd': {
> + double val = va_arg(args, double);
> + rc = virJSONValueObjectAppendNumberDouble(jargs, key, val);
> + } break;
> +
> + case 'B':
> + case 'b': {
> + int val = va_arg(args, int);
> +
> + if (!val && type == 'B')
> + continue;
> +
> + rc = virJSONValueObjectAppendBoolean(jargs, key, val);
> + } break;
> +
> + case 'n': {
> + rc = virJSONValueObjectAppendNull(jargs, key);
> + } break;
> +
> + case 'a': {
> + virJSONValuePtr val = va_arg(args, virJSONValuePtr);
> + rc = virJSONValueObjectAppend(jargs, key, val);
> + } break;
> +
> + default:
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unsupported data type '%c' for arg '%s'"), type, key - 2);
> + goto cleanup;
> + }
> +
> + if (rc < 0)
> + goto cleanup;
> + }
> +
> + /* verify that we added at least one key-value pair */
> + if (rc == -2) {
I didn't check any of the virJSAONValue* functions called, but if any
return -2 for whatever reason "sometime" in the future, then this logic
is flawed.
However, at this point you should have an array size/count right? And if
> 0, then we added something (eg, npairs count should be incremented)
thus virJSONValueObjectKeysNumber() should return > 0
> + ret = 0;
> + goto cleanup;
> + }
> +
> + *obj = jargs;
> + jargs = NULL;
> + ret = 1;
> +
> + cleanup:
> + virJSONValueFree(jargs);
> + return ret;
> +}
> +
> +
> +int
> +virJSONValueObjectCreate(virJSONValuePtr *obj, ...)
> +{
> + va_list args;
> + int ret;
> +
> + va_start(args, obj);
> + ret = virJSONValueObjectCreateVArgs(obj, args);
> + va_end(args);
> +
> + return ret;
> +}
> +
> +
> void
> virJSONValueFree(virJSONValuePtr value)
> {
> diff --git a/src/util/virjson.h b/src/util/virjson.h
> index 9487729..be603ae 100644
> --- a/src/util/virjson.h
> +++ b/src/util/virjson.h
> @@ -26,6 +26,8 @@
>
> # include "internal.h"
>
> +# include <stdarg.h>
> +
>
> typedef enum {
> VIR_JSON_TYPE_OBJECT,
> @@ -79,6 +81,9 @@ struct _virJSONValue {
>
> void virJSONValueFree(virJSONValuePtr value);
>
> +int virJSONValueObjectCreate(virJSONValuePtr *obj, ...);
> +int virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args);
Add an ATTRIBUTE_NONNULL(1) for this definition, since you deref in
function.
ACK with adjustments
John
> +
> virJSONValuePtr virJSONValueNewString(const char *data);
> virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length);
> virJSONValuePtr virJSONValueNewNumberInt(int data);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list