[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 24/28] qapi: Add JSON output visitor
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v4 24/28] qapi: Add JSON output visitor |
Date: |
Fri, 3 Jun 2016 06:53:21 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 06/03/2016 01:39 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> We have several places that want to go from qapi to JSON; right now,
>> they have to create an intermediate QObject to do the work. That
>> also has the drawback that the JSON formatting of a QDict will
>> rearrange keys (according to a deterministic, but unpredictable,
>> hash), when humans have an easier time if dicts are produced in
>> the same order as the qapi type.
>>
>> +struct JsonOutputVisitor {
>> + Visitor visitor;
>> + QString *str;
>> + bool comma;
>> + unsigned int depth;
>> + char **result;
>> +};
>> +
>> +static void json_output_complete(Visitor *v, void *result)
>> +{
>> + JsonOutputVisitor *jov = to_jov(v);
>> +
>> + assert(!jov->depth);
>> + assert(qstring_get_length(jov->str));
>> + assert(jov->result == result);
>> + *jov->result = qstring_consume_str(jov->str);
>> + jov->str = NULL;
>> + jov->result = NULL;
>> +}
>
> Related: discussion of complete() semantics in review of PATCH 12.
> Non-idempotent semantics save us a copy, like in
> string_output_complete().
>
>> +
>> +static void json_output_free(Visitor *v)
>> +{
>> + JsonOutputVisitor *jov = to_jov(v);
>> +
>> + QDECREF(jov->str);
>> + g_free(jov);
>
> I'm afraid this leaks jov->result when the visitor is destroyed without
> calling its complete() method.
Not true. jov->result is owned by the caller, and not something we
allocate locally. We set jov->result to NULL to make sure complete() is
not called twice, but we are not responsible for freeing it, since we
didn't allocate it.
>
> string_output_free() appears to have the same leak.
Same non-bug.
>
>> +}
>> +
>> +Visitor *json_output_visitor_new(char **result)
>> +{
>> + JsonOutputVisitor *v;
>> +
>> + v = g_malloc0(sizeof(*v));
>> + v->result = result;
>> + *result = NULL;
>> + v->str = qstring_new();
>> +
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature