Re: [libvirt] [PATCHv2] network: Avoid memory leaks on networkBuildDnsmasqArgv

On 01/18/2012 03:04 AM, ajia(a)redhat.com wrote: > From: Alex Jia&lt;ajia(a)redhat.com&gt; > > Detected by valgrind. Leaks introduced in commit 973af236. > > * src/network/bridge_driver.c: fix memory leaks on failure and successful path. > > * How to reproduce? > % make -C tests check TESTS=networkxml2argvtest > % cd tests&& valgrind -v --leak-check=full ./networkxml2argvtest > > src/network/bridge_driver.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) Needs a v3. > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 5d0d528..5bd5a50 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -459,6 +459,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, > int r, ret = -1; > int nbleases = 0; > int ii; > + char *recordPort = NULL; > + char *recordPriority = NULL; > + char *recordWeight = NULL; > virNetworkIpDefPtr tmpipdef; Moving the declaration here, and a cleanup at the end, will only free _one_ instance of allocation. > > /* > @@ -530,9 +533,6 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, > > for (i = 0; i< dns->nsrvrecords; i++) { > char *record = NULL; > - char *recordPort = NULL; > - char *recordPriority = NULL; > - char *recordWeight = NULL; But these values were allocated in a for loop, so if there is more than one nsrvrecords, then you are leaking on each iteration of the loop. > > if (dns->srvrecords[i].service&& dns->srvrecords[i].protocol) { > if (dns->srvrecords[i].port) { > @@ -671,6 +671,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, > > ret = 0; > cleanup: > + VIR_FREE(recordPort); > + VIR_FREE(recordWeight); > + VIR_FREE(recordPriority); You need to also copy these three lines into the end of the for loop.