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

2012-01-19 Thread Eric Blake 
On 01/18/2012 03:04 AM, a...@redhat.com wrote:
 From: Alex Jia a...@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.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-01-18 Thread ajia 
From: Alex Jia a...@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

* 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 a...@redhat.com
---
 src/network/bridge_driver.c |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

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;
 
 /*
@@ -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;
 
 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);
 return ret;
 }
 
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list