On Wed, Jan 27, 2010 at 11:35:15AM +0100, Jim Meyering wrote: > Daniel P. Berrange wrote: > > On Wed, Jan 27, 2010 at 11:17:32AM +0100, Jim Meyering wrote: > >> Daniel P. Berrange wrote: > >> > On Wed, Jan 27, 2010 at 10:38:55AM +0100, Jim Meyering wrote: > >> >> Actually, the preceding patch fixed only the one leak that had been > >> >> introduced in the last month or so. Looking at the many other functions > >> >> that do the same sort of thing (call qemuMonitorJSONMakeCommand, and > >> >> later virJSONValueFree), I saw that they all had exactly the same leak. > >> >> So this amended patch fixes all of them: > >> >> > >> >> >From 28f820354dcae9950cad042ea78a893fd9475830 Mon Sep 17 00:00:00 2001 > >> >> From: Jim Meyering <meyering@xxxxxxxxxx> > >> >> Date: Wed, 27 Jan 2010 09:58:12 +0100 > >> >> Subject: [PATCH] qemu_monitor_json.c: avoid many unconditional leaks > >> > > >> > The real bug is the virJSONValueFree() itself which is missing > >> > the final VIR_FREE(value) call. By doing the free in the caller, we > >> > still leak data for compound array/hash values. > >> > >> Putting the VIR_FREE in virJSONValueFree was my first reflex, too, > >> but since coverity detected no leak for the adjacent > >> "virJSONValueFree(reply);" use, I assumed that doing so would > >> cause a problem: > >> > >> virJSONValueFree(cmd); > >> VIR_FREE(cmd); > >> virJSONValueFree(reply); > > > > I think coverity must be confused then :-) The virJSONValueFree() method > > is definitely intended to free any memory allocated during one of the > > virJSONValueNewXXXX() methods. > > :-) > In that case, here's a replacement patch: > > >From 96196ca0b0cd71ae6b7f2dd7668432db95678e70 Mon Sep 17 00:00:00 2001 > From: Jim Meyering <meyering@xxxxxxxxxx> > Date: Wed, 27 Jan 2010 09:58:12 +0100 > Subject: [PATCH] json.c: avoid an unconditional leak from most qemuMonitorJSON* functions > > * src/util/json.c (virJSONValueFree): Free the "value" pointer, too. > --- > src/util/json.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/util/json.c b/src/util/json.c > index a292e1b..1b3c359 100644 > --- a/src/util/json.c > +++ b/src/util/json.c > @@ -2,7 +2,7 @@ > * json.c: JSON object parsing/formatting > * > * Copyright (C) 2009 Daniel P. Berrange > - * Copyright (C) 2009 Red Hat, Inc. > + * Copyright (C) 2009-2010 Red Hat, Inc. > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -82,8 +82,9 @@ void virJSONValueFree(virJSONValuePtr value) > case VIR_JSON_TYPE_NUMBER: > VIR_FREE(value->data.number); > break; > - > } > + > + VIR_FREE(value); > } > > ACK, looks fine to me, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list