On 06/03/2016 01:39 AM, Markus Armbruster wrote: > Eric Blake <ebl...@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
signature.asc
Description: OpenPGP digital signature
