Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion

[Michael Roth]

On Mon, Aug 13, 2012 at 09:25:44PM -0500, Michael Roth wrote:
> On Mon, Aug 13, 2012 at 03:01:56PM -0300, Luiz Capitulino wrote:
> > On Fri, 10 Aug 2012 18:24:10 -0500
> > Michael Roth <address@hidden> wrote:
> > 
> > > +    qlist_iter(tokens, tokens_count_from_iter, &count);
> > > +    if (count == 0) {
> > > +        return NULL;
> > > +    }
> > 
> > Please, add qlist_size() instead.
> > 
> 
> Gladly :) I spent a good amount for a function that did this, not sure
> how I missed it.
> 

Heh, read that as "please, use qlist_size() instead". I've added it
after further searching for it :)

> > > +
> > > +    ctxt = g_malloc0(sizeof(JSONParserContext));
> > > +    ctxt->tokens.pos = 0;
> > > +    ctxt->tokens.count = count;
> > > +    ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
> > > +    qlist_iter(tokens, tokens_append_from_iter, ctxt);
> > > +    ctxt->tokens.pos = 0;
> > > +
> > > +    return ctxt;
> > > +}
> > > +
> > > +static void parser_context_free(JSONParserContext *ctxt)
> > > +{
> > > +    int i;
> > > +    if (ctxt) {
> > > +        for (i = 0; i < ctxt->tokens.count; i++) {
> > > +            qobject_decref(ctxt->tokens.buf[i]);
> > > +        }
> > > +        g_free(ctxt->tokens.buf);
> > > +        g_free(ctxt);
> > 
> > Isn't this leaking ctxt->err?
> > 
> 
> Indeed. Looks like we were leaking this previously as well. I'll get it
> in V2.
> 

Apparently not, actually. It looks like error_propagate() handles
whether to free the error or pass it up the stack, so we can't mess with
it in the free function. I've added a comment to note that ctxt->err
needs to be freed seperately.

V2 is incoming with comments addressed.