From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:36281)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <flukshun@gmail.com>) id 1T1hjp-00047n-0p
	for qemu-devel@nongnu.org; Wed, 15 Aug 2012 13:50:41 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <flukshun@gmail.com>) id 1T1hjj-0008HU-Pq
	for qemu-devel@nongnu.org; Wed, 15 Aug 2012 13:50:36 -0400
Received: from mail-ob0-f173.google.com ([209.85.214.173]:64268)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <flukshun@gmail.com>) id 1T1hjj-0008HA-Kq
	for qemu-devel@nongnu.org; Wed, 15 Aug 2012 13:50:31 -0400
Received: by obbta14 with SMTP id ta14so2324199obb.4
	for <qemu-devel@nongnu.org>; Wed, 15 Aug 2012 10:50:30 -0700 (PDT)
Sender: fluxion <flukshun@gmail.com>
Date: Wed, 15 Aug 2012 12:50:06 -0500
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Message-ID: <20120815175006.GI16157@illuin>
References: <1344641050-21997-1-git-send-email-mdroth@linux.vnet.ibm.com>
	<20120813150156.4ad30188@doriath.home>
	<20120814022544.GE16157@illuin>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20120814022544.GE16157@illuin>
Subject: Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at
 each level of recursion
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org

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 <mdroth@linux.vnet.ibm.com> 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.