[libvirt] [PATCHv3 03/13] Switch from yajl to Jansson

Peter Krempa pkrempa at redhat.com
Tue May 15 13:05:55 UTC 2018


On Fri, May 11, 2018 at 14:59:04 +0200, Ján Tomko wrote:
> Yajl has not seen much activity upstream recently.

[0]

> 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 at redhat.com>
> ---
>  src/util/virjson.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 211 insertions(+)
> 
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 0559d40b64..2f7d624bb3 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -2008,6 +2008,217 @@ virJSONValueToString(virJSONValuePtr object,

[...]

> +static json_t *
> +virJSONValueToJansson(virJSONValuePtr object)
> +{
> +    json_t *ret = NULL;
> +    size_t i;
> +
> +    switch ((virJSONType)object->type) {
> +    case VIR_JSON_TYPE_OBJECT:
> +        ret = json_object();
> +        if (!ret) {
> +            virReportOOMError();
> +            goto error;
> +        }
> +        for (i = 0; i < object->data.object.npairs; i++) {
> +            virJSONObjectPairPtr cur = object->data.object.pairs + i;
> +            json_t *val = virJSONValueToJansson(cur->value);
> +
> +            if (!val)
> +                goto error;
> +            if (json_object_set_new(ret, cur->key, val) < 0) {
> +                virReportOOMError();

'val' is leaked here.

> +                goto error;
> +            }
> +        }
> +        break;
> +
> +    case VIR_JSON_TYPE_ARRAY:
> +        ret = json_array();
> +        if (!ret) {
> +            virReportOOMError();
> +            goto error;
> +        }
> +        for (i = 0; i < object->data.array.nvalues; i++) {
> +            virJSONValuePtr cur = object->data.array.values[i];
> +            json_t *val = virJSONValueToJansson(cur);
> +
> +            if (!val)
> +                goto error;
> +            if (json_array_append_new(ret, val) < 0) {
> +                virReportOOMError();

'val' is leaked here.

> +                goto error;
> +            }
> +        }
> +        break;
> +
> +    case VIR_JSON_TYPE_STRING:
> +        ret = json_string(object->data.string);
> +        break;
> +
> +    case VIR_JSON_TYPE_NUMBER: {
> +        long long ll_val;
> +        double d_val;
> +        if (virStrToLong_ll(object->data.number, NULL, 10, &ll_val) < 0) {
> +            if (virStrToDouble(object->data.number, NULL, &d_val) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("JSON value is not a number"));
> +                return NULL;
> +            }
> +            ret = json_real(d_val);
> +        } else {
> +            ret = json_integer(ll_val);
> +        }
> +    }
> +        break;
> +
> +    case VIR_JSON_TYPE_BOOLEAN:
> +        ret = json_boolean(object->data.boolean);
> +        break;
> +
> +    case VIR_JSON_TYPE_NULL:
> +        ret = json_null();
> +        break;
> +
> +    default:
> +        virReportEnumRangeError(virJSONType, object->type);
> +        goto error;
> +    }
> +    if (!ret)
> +        virReportOOMError();
> +    return ret;

Few lines could be saved by having a dedicated label for cases when
virReportOOMError needs to be called.

> +
> + error:
> +    json_decref(ret);
> +    return NULL;

ACK if you fix the leaks. The separate label is up to you. Don't forget
to update the commit message after the squash-in.

Peter

[0] For anyone following current meme trends, this looks like it's
relevant to our YAJL->janson switch:

https://i.redditmedia.com/J46fZN24lFx3fMRlJNpkNEOFqU79zWTsRDBMla1u0HE.jpg?s=4757d31d1cbd758962407917e0d3aacb
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180515/e250ac5f/attachment-0001.sig>


More information about the libvir-list mailing list