On Sat, Mar 31, 2018 at 11:13:13AM +0200, Peter Krempa wrote:
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@redhat.com> --- 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.
Note the second copy will drop back down to 1 copy when we later drop YAJL support, so this is not a long term problem.
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.
That means that we are basically reinventing JSON formatting & escaping rules in our code. I don't think that would be a step forward. I wish we could someday get rid of our use of virBuffer for formatting XML too and rely on a XML library for formatting just as we do for JSON. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|