Re: [PATCHv2 05/12] Switch from yajl to Jansson

[Peter Krempa <pkrempa@xxxxxxxxxx>]

On Thu, May 10, 2018 at 18:44:17 +0200, Ján Tomko wrote:
> Yajl has not seen much activity upstream recently.
> Switch to using Jansson >= 2.7.
> 
> All the platforms we target on https://libvirt.org/platforms.html
> have a version >= 2.7 listed on the sites below:
> https://repology.org/metapackage/jansson/versions
> https://build.opensuse.org/package/show/devel:libraries:c_c++/libjansson
> 
> Implement virJSONValue{From,To}String using Jansson, delete the yajl
> code (and the related virJSONParser structure) and report an error
> if someone explicitly specifies --with-yajl.
> 
> Also adjust the test data to account for Jansson's different whitespace
> usage for empty arrays and tune up the specfile to keep 'make rpm'
> working when bisecting.
> 
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
>  src/util/virjson.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 223 insertions(+)
> 
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 92a15b28a1..cb38c35522 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -2011,6 +2011,229 @@ 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) {
> +                virJSONValueFree(val);
> +                goto error;
> +            }
> +        }
> +
> +        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) {
> +                virJSONValueFree(val);
> +                goto error;
> +            }
> +        }
> +        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;
> +
> +    case JSON_TRUE:
> +    case JSON_FALSE:
> +        ret = virJSONValueNewBoolean(json_boolean_value(json));
> +        break;
> +
> +    case JSON_NULL:
> +        ret = virJSONValueNewNull();
> +        break;

Maybe add a 'default' case?

> +    }
> +
> +    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;

This is not initialized or used with jansson ...

> +    json_t *ret = NULL;
> +    size_t i;
> +
> +    switch (object->type) {
> +    case VIR_JSON_TYPE_OBJECT:
> +        ret = json_object();
> +        if (!ret) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("failed to create JSON object: %s"),
> +                           error.text);

... so here you'll try to print an uninitialized pointer.


> +            goto error;
> +        }

[...]

All of the truncated ones have the same problem.


> +    }
> +    if (!ret) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("error creating JSON value: %s"),
> +                       error.text);
> +    }
> +    return ret;
> +
> + error:
> +    json_decref(ret);
> +    return NULL;
> +}
> +
> +
> +char *
> +virJSONValueToString(virJSONValuePtr object,
> +                     bool pretty)
> +{
> +    size_t flags = JSON_ENCODE_ANY;
> +    json_t *json;
> +    json_error_t error;
> +    char *str = NULL;
> +
> +    if (pretty)
> +        flags |= JSON_INDENT(2);
> +    else
> +        flags |= JSON_COMPACT;
> +
> +    json = virJSONValueToJansson(object);
> +    if (!json)
> +        return NULL;
> +
> +    str = json_dumps(json, flags);
> +    if (!str) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("failed to format JSON %d:%d: %s"),
> +                       error.line, error.column, error.text);

Same problem.

> +    }
> +    json_decref(json);
> +    return str;

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list