Re: [libvirt] [PATCH 8/9] virjson: add support for Jansson

On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote: > Check for the presence of Jansson library and prefer it to yajl > if possible. > > The minimum required version is 2.7. > > Internally, virJSONValue still stores numbers as strings even > though Jansson uses numeric variables for them. > > The configure script is particularly hideous, but will hopefully > go away after we stop aiming to support compiling on CentOS 6. > > Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; > --- > configure.ac | 1 + > m4/virt-json.m4 | 55 +++++++++++--- > src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 264 insertions(+), 11 deletions(-) > diff --git a/m4/virt-json.m4 b/m4/virt-json.m4 > index 5e4bcc7c9..a5ae3edcd 100644 > --- a/m4/virt-json.m4 > +++ b/m4/virt-json.m4 > diff --git a/src/util/virjson.c b/src/util/virjson.c > index 6a02ddf0c..86cbd6eef 100644 > --- a/src/util/virjson.c > +++ b/src/util/virjson.c > @@ -1951,6 +1951,225 @@ virJSONValueToString(virJSONValuePtr object, > } > > > +#elif WITH_JANSSON > +# include <jansson.h> > + > +static virJSONValuePtr > +virJSONValueFromJansson(json_t *json) > +{ > + virJSONValuePtr ret = NULL; > + const char *key; > + json_t *cur; > + size_t i; > + > + switch (json_typeof(json)) { > + case JSON_OBJECT: > + ret = virJSONValueNewObject(); > + if (!ret) > + goto error; > + > + json_object_foreach(json, key, cur) { > + virJSONValuePtr val = virJSONValueFromJansson(cur); > + if (!val) > + goto error; > + > + if (virJSONValueObjectAppend(ret, key, val) < 0) > + goto error; 'val' will be leaked on failure > + } > + > + break; > + > + case JSON_ARRAY: > + ret = virJSONValueNewArray(); > + if (!ret) > + goto error; > + > + json_array_foreach(json, i, cur) { > + virJSONValuePtr val = virJSONValueFromJansson(cur); > + if (!val) > + goto error; > + > + if (virJSONValueArrayAppend(ret, val) < 0) > + goto error; here too > + } > + break; > + > + case JSON_STRING: > + ret = virJSONValueNewStringLen(json_string_value(json), > + json_string_length(json)); > + break; > + > + case JSON_INTEGER: > + ret = virJSONValueNewNumberLong(json_integer_value(json)); > + break; > + > + case JSON_REAL: > + ret = virJSONValueNewNumberDouble(json_real_value(json)); > + break; After mi privatization of struct _virJSONValue it should be simple enough to add the same differetiation to our code. Not sure whether that's worth though. > + > + case JSON_TRUE: > + case JSON_FALSE: > + ret = virJSONValueNewBoolean(json_boolean_value(json)); > + break; > + > + case JSON_NULL: > + ret = virJSONValueNewNull(); > + break; > + } > + > + return ret; > + > + error: > + virJSONValueFree(ret); > + return NULL; > +} > + > +virJSONValuePtr > +virJSONValueFromString(const char *jsonstring) > +{ > + virJSONValuePtr ret = NULL; > + json_t *json; > + json_error_t error; > + size_t flags = JSON_REJECT_DUPLICATES | > + JSON_DECODE_ANY; > + > + if (!(json = json_loads(jsonstring, flags, &error))) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("failed to parse JSON %d:%d: %s"), > + error.line, error.column, error.text); > + return NULL; > + } > + > + ret = virJSONValueFromJansson(json); > + json_decref(json); > + return ret; > +} > + > + > +static json_t * > +virJSONValueToJansson(virJSONValuePtr object) > +{ > + json_error_t error; > + json_t *ret = NULL; > + size_t i; > + > + switch (object->type) { > + case VIR_JSON_TYPE_OBJECT: > + ret = json_object(); > + if (!ret) { So this would be the second copy of a similar function. I propose that we replace the formatter which in this case copies everything from our structs into structs of janson to format it with a formatter which directly uses virBuffer to do so.

https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html This will allow us to have a single copy of the formatter and additionally it will not depend on the library.