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@xxxxxxxxxx>
> > ---
> > 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
