[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]