Re: [Qemu-devel] [PATCH v4 24/28] qapi: Add JSON output visitor

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]

On 06/03/2016 01:39 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]