[libvirt] PATCH: Fix some more memory leaks

[Daniel P. Berrange]

While testing Cole's series of patches I identified a couple more places
where we leak memory.

In libvirt.c, the default authentication callback uses uninitialized
data, and indeed strdup()'s it and this is then never released. This
simply disables that bit of code.

In qparams.c when free'ing the struct the 'p' struct field was not
released. I took the opportunity to switch it over to using the new
style memory.h functions

In remote.c there were  a couple of handlers which forgot to free the
virDomainPtr object when they were done.

 qemud/remote.c |   18 ++++++++++++++----
 src/libvirt.c  |   16 +++++++++-------
 src/qparams.c  |   31 +++++++++++++------------------
 3 files changed, 36 insertions(+), 29 deletions(-)


Dan.

Index: qemud/remote.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/remote.c,v
retrieving revision 1.32
diff -u -p -r1.32 remote.c
--- qemud/remote.c	21 May 2008 20:53:30 -0000	1.32
+++ qemud/remote.c	21 May 2008 21:16:42 -0000
@@ -792,8 +792,11 @@ remoteDispatchDomainBlockStats (struct q
     }
     path = args->path;
 
-    if (virDomainBlockStats (dom, path, &stats, sizeof stats) == -1)
+    if (virDomainBlockStats (dom, path, &stats, sizeof stats) == -1) {
+        virDomainFree (dom);
         return -1;
+    }
+    virDomainFree (dom);
 
     ret->rd_req = stats.rd_req;
     ret->rd_bytes = stats.rd_bytes;
@@ -823,8 +826,11 @@ remoteDispatchDomainInterfaceStats (stru
     }
     path = args->path;
 
-    if (virDomainInterfaceStats (dom, path, &stats, sizeof stats) == -1)
+    if (virDomainInterfaceStats (dom, path, &stats, sizeof stats) == -1) {
+        virDomainFree (dom);
         return -1;
+    }
+    virDomainFree (dom);
 
     ret->rx_bytes = stats.rx_bytes;
     ret->rx_packets = stats.rx_packets;
@@ -1258,7 +1264,11 @@ remoteDispatchDomainMigratePerform (stru
                                    args->cookie.cookie_len,
                                    args->uri,
                                    args->flags, dname, args->resource);
-    if (r == -1) return -1;
+    if (r == -1) {
+        virDomainFree (dom);
+        return -1;
+    }
+    virDomainFree (dom);
 
     return 0;
 }
@@ -1281,7 +1291,7 @@ remoteDispatchDomainMigrateFinish (struc
     if (ddom == NULL) return -1;
 
     make_nonnull_domain (&ret->ddom, ddom);
-
+    virDomainFree (ddom);
     return 0;
 }
 
Index: src/libvirt.c
===================================================================
RCS file: /data/cvs/libvirt/src/libvirt.c,v
retrieving revision 1.140
diff -u -p -r1.140 libvirt.c
--- src/libvirt.c	21 May 2008 20:53:31 -0000	1.140
+++ src/libvirt.c	21 May 2008 21:16:45 -0000
@@ -170,13 +170,15 @@ static int virConnectAuthCallbackDefault
             return -1;
         }
 
-        if (STREQ(bufptr, "") && cred[i].defresult)
-            cred[i].result = strdup(cred[i].defresult);
-        else
-            cred[i].result = strdup(bufptr);
-        if (!cred[i].result)
-            return -1;
-        cred[i].resultlen = strlen(cred[i].result);
+        if (cred[i].type != VIR_CRED_EXTERNAL) {
+            if (STREQ(bufptr, "") && cred[i].defresult)
+                cred[i].result = strdup(cred[i].defresult);
+            else
+                cred[i].result = strdup(bufptr);
+            if (!cred[i].result)
+                return -1;
+            cred[i].resultlen = strlen(cred[i].result);
+        }
     }
 
     return 0;
Index: src/qparams.c
===================================================================
RCS file: /data/cvs/libvirt/src/qparams.c,v
retrieving revision 1.4
diff -u -p -r1.4 qparams.c
--- src/qparams.c	28 Apr 2008 15:14:59 -0000	1.4
+++ src/qparams.c	21 May 2008 21:16:45 -0000
@@ -27,7 +27,7 @@
 #include <stdarg.h>
 
 #include "buf.h"
-
+#include "memory.h"
 #include "qparams.h"
 
 struct qparam_set *
@@ -39,13 +39,12 @@ new_qparam_set (int init_alloc, ...)
 
     if (init_alloc <= 0) init_alloc = 1;
 
-    ps = malloc (sizeof (*ps));
-    if (!ps) return NULL;
+    if (VIR_ALLOC(ps) < 0)
+        return NULL;
     ps->n = 0;
     ps->alloc = init_alloc;
-    ps->p = malloc (init_alloc * sizeof (ps->p[0]));
-    if (!ps->p) {
-        free (ps);
+    if (VIR_ALLOC_N(ps->p, ps->alloc) < 0) {
+        VIR_FREE (ps);
         return NULL;
     }
 
@@ -87,13 +86,8 @@ append_qparams (struct qparam_set *ps, .
 static int
 grow_qparam_set (struct qparam_set *ps)
 {
-    struct qparam *old_p;
-
     if (ps->n >= ps->alloc) {
-        old_p = ps->p;
-        ps->p = realloc (ps->p, 2 * ps->alloc * sizeof (ps->p[0]));
-        if (!ps->p) {
-            ps->p = old_p;
+        if (VIR_REALLOC_N(ps->p, ps->alloc * 2) < 0) {
             perror ("realloc");
             return -1;
         }
@@ -115,13 +109,13 @@ append_qparam (struct qparam_set *ps,
 
     pvalue = strdup (value);
     if (!pvalue) {
-        free (pname);
+        VIR_FREE (pname);
         return -1;
     }
 
     if (grow_qparam_set (ps) == -1) {
-        free (pname);
-        free (pvalue);
+        VIR_FREE (pname);
+        VIR_FREE (pvalue);
         return -1;
     }
 
@@ -161,10 +155,11 @@ free_qparam_set (struct qparam_set *ps)
     int i;
 
     for (i = 0; i < ps->n; ++i) {
-        free (ps->p[i].name);
-        free (ps->p[i].value);
+        VIR_FREE (ps->p[i].name);
+        VIR_FREE (ps->p[i].value);
     }
-    free (ps);
+    VIR_FREE (ps->p);
+    VIR_FREE (ps);
 }
 
 struct qparam_set *


-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|