On 2012年01月06日 13:14, 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? % cd tests&& valgrind -v --leak-check=full ./networkxml2argvtest
* Actual result:
==2226== 3 bytes in 1 blocks are definitely lost in loss record 1 of 24 ==2226== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==2226== by 0x39CF0FEDE7: __vasprintf_chk (in /lib64/libc-2.12.so) ==2226== by 0x41DFF7: virVasprintf (stdio2.h:199) ==2226== by 0x41E0B7: virAsprintf (util.c:1695) ==2226== by 0x41A2D9: networkBuildDhcpDaemonCommandLine (bridge_driver.c:545) ==2226== by 0x4145C8: testCompareXMLToArgvHelper (networkxml2argvtest.c:47) ==2226== by 0x4156A1: virtTestRun (testutils.c:141) ==2226== by 0x414332: mymain (networkxml2argvtest.c:123) ==2226== by 0x414D97: virtTestMain (testutils.c:696) ==2226== by 0x39CF01ECDC: (below main) (in /lib64/libc-2.12.so) ==2226== ==2226== 3 bytes in 1 blocks are definitely lost in loss record 2 of 24 ==2226== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==2226== by 0x39CF0FEDE7: __vasprintf_chk (in /lib64/libc-2.12.so) ==2226== by 0x41DFF7: virVasprintf (stdio2.h:199) ==2226== by 0x41E0B7: virAsprintf (util.c:1695) ==2226== by 0x41A307: networkBuildDhcpDaemonCommandLine (bridge_driver.c:551) ==2226== by 0x4145C8: testCompareXMLToArgvHelper (networkxml2argvtest.c:47) ==2226== by 0x4156A1: virtTestRun (testutils.c:141) ==2226== by 0x414332: mymain (networkxml2argvtest.c:123) ==2226== by 0x414D97: virtTestMain (testutils.c:696) ==2226== by 0x39CF01ECDC: (below main) (in /lib64/libc-2.12.so) ==2226== ==2226== 5 bytes in 1 blocks are definitely lost in loss record 4 of 24 ==2226== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==2226== by 0x39CF0FEDE7: __vasprintf_chk (in /lib64/libc-2.12.so) ==2226== by 0x41DFF7: virVasprintf (stdio2.h:199) ==2226== by 0x41E0B7: virAsprintf (util.c:1695) ==2226== by 0x41A2AB: networkBuildDhcpDaemonCommandLine (bridge_driver.c:539) ==2226== by 0x4145C8: testCompareXMLToArgvHelper (networkxml2argvtest.c:47) ==2226== by 0x4156A1: virtTestRun (testutils.c:141) ==2226== by 0x414332: mymain (networkxml2argvtest.c:123) ==2226== by 0x414D97: virtTestMain (testutils.c:696) ==2226== by 0x39CF01ECDC: (below main) (in /lib64/libc-2.12.so) ==2226== ==2226== LEAK SUMMARY: ==2226== definitely lost: 11 bytes in 3 blocks
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/network/bridge_driver.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9afada7..63a28a6 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -544,12 +544,15 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, if (dns->srvrecords[i].priority) { if (virAsprintf(&recordPriority, "%d", dns->srvrecords[i].priority)< 0) { virReportOOMError(); + VIR_FREE(recordPort); goto cleanup; } } if (dns->srvrecords[i].weight) { if (virAsprintf(&recordWeight, "%d", dns->srvrecords[i].weight)< 0) { virReportOOMError(); + VIR_FREE(recordPort); + VIR_FREE(recordPriority); goto cleanup; } } @@ -567,6 +570,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, }
virCommandAddArgPair(cmd, "--srv-host", record); + VIR_FREE(recordPort); + VIR_FREE(recordPriority); + VIR_FREE(recordWeight); VIR_FREE(record); } }
These fixes are right, but you might also want to free() "recordPort", "recordPriority", and "recordWeight" before "goto cleanup" in following branch: if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", dns->srvrecords[i].service, dns->srvrecords[i].protocol, dns->srvrecords[i].domain ? dns->srvrecords[i].domain : "", dns->srvrecords[i].target ? dns->srvrecords[i].target : "", recordPort ? recordPort : "", recordPriority ? recordPriority : "", recordWeight ? recordWeight : "") < 0) { And personally I'd use "goto", too much duplicate VIR_FREE use. Regards, Osier