On 01/20/2012 04:43 AM, Eric Blake wrote:
On 01/18/2012 03:04 AM, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
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.
Yeah, these values were allocated in a for loop, so also should free them in a loop, thanks for your comment, and will commit a v3 now. Regards, Alex