On 2012年01月06日 13:14, 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? > % 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&lt;ajia(a)redhat.com&gt; > --- > 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