From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44325) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL9G1-0005SU-L9 for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:36:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UL9Fz-00065y-SD for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:36:29 -0400 Date: Thu, 28 Mar 2013 10:36:09 +0100 From: Stefan Hajnoczi Message-ID: <20130328093609.GD24910@stefanha-thinkpad.redhat.com> References: <5152174F.6080908@linux.vnet.ibm.com> <20130327214715.GD8146@vm> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130327214715.GD8146@vm> Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix some memory leaks in test-visitor-serialization List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mdroth Cc: qemu-trivial@nongnu.org, qemu-devel , Stefan Berger On Wed, Mar 27, 2013 at 04:47:15PM -0500, mdroth wrote: > On Tue, Mar 26, 2013 at 05:46:55PM -0400, Stefan Berger wrote: > > This patch fixes some of the memory leaks in > > test-visitor-serialization but not all of them. > > > > Signed-off-by: Stefan Berger > > > > --- > > tests/test-visitor-serialization.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > Index: qemu-git.pt/tests/test-visitor-serialization.c > > =================================================================== > > --- qemu-git.pt.orig/tests/test-visitor-serialization.c > > +++ qemu-git.pt/tests/test-visitor-serialization.c > > @@ -258,6 +258,7 @@ static void test_primitives(gconstpointe > > g_assert(pt_copy != NULL); > > if (pt->type == PTYPE_STRING) { > > g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string); > > + g_free((char *)pt_copy->value.string); > > } else if (pt->type == PTYPE_NUMBER) { > > /* we serialize with %f for our reference visitors, so > > rather than fuzzy > > * floating math to test "equality", just compare the > > formatted values > > @@ -275,6 +276,7 @@ static void test_primitives(gconstpointe > > > > ops->cleanup(serialize_data); > > g_free(args); > > + g_free(pt_copy); > > } > > > > static void test_struct(gconstpointer opaque) > > @@ -637,6 +639,7 @@ PrimitiveType pt_values[] = { > > /* visitor-specific op implementations */ > > > > typedef struct QmpSerializeData { > > + QObject *obj; > > QmpOutputVisitor *qov; > > QmpInputVisitor *qiv; > > } QmpSerializeData; > > @@ -656,10 +659,10 @@ static void qmp_deserialize(void **nativ > > { > > QmpSerializeData *d = datap; > > QString *output_json = > > qobject_to_json(qmp_output_get_qobject(d->qov)); > > - QObject *obj = qobject_from_json(qstring_get_str(output_json)); > > > > + d->obj = qobject_from_json(qstring_get_str(output_json)); > > QDECREF(output_json); > > - d->qiv = qmp_input_visitor_new(obj); > > + d->qiv = qmp_input_visitor_new(d->obj); > > visit(qmp_input_get_visitor(d->qiv), native_out, errp); > > } > > > > @@ -668,9 +671,13 @@ static void qmp_cleanup(void *datap) > > QmpSerializeData *d = datap; > > qmp_output_visitor_cleanup(d->qov); > > qmp_input_visitor_cleanup(d->qiv); > > + qobject_decref(d->obj); > > We can actually call this just after passing the qobject to > qmp_input_visitor_new(), which will lead to it getting cleaned up > in qmp_input_visitor_cleanup(). That way we wouldn't need to hold > on to the reference in d->obj. > > But this works too, and looks like string visitor still requires > we hang on to the reference to free it, so either way: > > Reviewed-by: Michael Roth Please decref right away like mdroth suggests so the obj field doesn't need to be introduced. Stefan