Re: PATCH: 8/10: Fix misc mem leaks

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

Testing with valgrind found a number of memory leaks in all
sorts of places in the drivers and virsh.

When freeing a virConnect object, we don't clear any set
virError object, potentially leaking the message strings.

When removing IPtables rules we inexplicably strdup() one
of the params to strcmp.

We fail to free the TLS x509 certificate directory path
when shutting down the QEMU driver

When the remote driver gets an error back from the server
for some reason it strdup()s the strings passed into the
__virRaiseError function, and then fails to free them.

When virsh shuts down it doesn't clear the global error
variable, nor free its URI string, and sometimes fails
to release the virConnectPtr object


 hash.c            |    1 +
 iptables.c        |    2 +-
 qemu_driver.c     |    3 +++
 remote_internal.c |   15 ++++-----------
 virsh.c           |   15 +++++++++++----
 5 files changed, 20 insertions(+), 16 deletions(-)



diff -r efdf8bc4ae07 src/hash.c
--- a/src/hash.c	Mon Nov 26 09:58:42 2007 -0500
+++ b/src/hash.c	Mon Nov 26 10:20:17 2007 -0500
@@ -726,6 +726,7 @@ virFreeConnect(virConnectPtr conn) {
         virHashFree(conn->networks, (virHashDeallocator) virNetworkFreeName);
     if (conn->hashes_mux != NULL)
         xmlFreeMutex(conn->hashes_mux);
+    virResetError(&conn->err);
     free(conn);
     return(0);
 }
diff -r efdf8bc4ae07 src/iptables.c
--- a/src/iptables.c	Mon Nov 26 09:58:42 2007 -0500
+++ b/src/iptables.c	Mon Nov 26 10:20:17 2007 -0500
@@ -246,7 +246,7 @@ iptRulesRemove(iptRules *rules,
     int i;
 
     for (i = 0; i < rules->nrules; i++)
-        if (!strcmp(rules->rules[i].rule, strdup(rule)))
+        if (!strcmp(rules->rules[i].rule, rule))
             break;
 
     if (i >= rules->nrules)
diff -r efdf8bc4ae07 src/qemu_driver.c
--- a/src/qemu_driver.c	Mon Nov 26 09:58:42 2007 -0500
+++ b/src/qemu_driver.c	Mon Nov 26 10:20:17 2007 -0500
@@ -338,6 +338,9 @@ qemudShutdown(void) {
         free(qemu_driver->networkConfigDir);
     if (qemu_driver->networkAutostartDir)
         free(qemu_driver->networkAutostartDir);
+
+    if (qemu_driver->vncTLSx509certdir)
+        free(qemu_driver->vncTLSx509certdir);
 
     if (qemu_driver->brctl)
         brShutdown(qemu_driver->brctl);
diff -r efdf8bc4ae07 src/remote_internal.c
--- a/src/remote_internal.c	Mon Nov 26 09:58:42 2007 -0500
+++ b/src/remote_internal.c	Mon Nov 26 10:20:17 2007 -0500
@@ -3808,20 +3808,13 @@ server_error (virConnectPtr conn, remote
     dom = err->dom ? get_nonnull_domain (conn, *err->dom) : NULL;
     net = err->net ? get_nonnull_network (conn, *err->net) : NULL;
 
-    /* These strings are nullable.  OK to ignore the return value
-     * of strdup since these strings are informational.
-     */
-    char *str1 = err->str1 ? strdup (*err->str1) : NULL;
-    char *str2 = err->str2 ? strdup (*err->str2) : NULL;
-    char *str3 = err->str3 ? strdup (*err->str3) : NULL;
-
-    char *message = err->message ? strdup (*err->message) : NULL;
-
     __virRaiseError (conn, dom, net,
                      err->domain, err->code, err->level,
-                     str1, str2, str3,
+                     err->str1 ? *err->str1 : NULL,
+                     err->str2 ? *err->str2 : NULL,
+                     err->str3 ? *err->str3 : NULL,
                      err->int1, err->int2,
-                     "%s", message);
+                     "%s", err->message ? *err->message : NULL);
 }
 
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
diff -r efdf8bc4ae07 src/virsh.c
--- a/src/virsh.c	Mon Nov 26 09:58:42 2007 -0500
+++ b/src/virsh.c	Mon Nov 26 10:20:17 2007 -0500
@@ -4779,7 +4779,8 @@ vshDeinit(vshControl * ctl)
 vshDeinit(vshControl * ctl)
 {
     vshCloseLogFile(ctl);
-
+    if (ctl->name)
+        free(ctl->name);
     if (ctl->conn) {
         if (virConnectClose(ctl->conn) != 0) {
             ctl->conn = NULL;   /* prevent recursive call from vshError() */
@@ -4787,6 +4788,8 @@ vshDeinit(vshControl * ctl)
                      "failed to disconnect from the hypervisor");
         }
     }
+    virResetLastError();
+
     return TRUE;
 }
 
@@ -4984,11 +4987,15 @@ main(int argc, char **argv)
         ctl->name = strdup(defaultConn);
     }
 
-    if (!vshParseArgv(ctl, argc, argv))
+    if (!vshParseArgv(ctl, argc, argv)) {
+        vshDeinit(ctl);
         exit(EXIT_FAILURE);
-
-    if (!vshInit(ctl))
+    }
+
+    if (!vshInit(ctl)) {
+        vshDeinit(ctl);
         exit(EXIT_FAILURE);
+    }
 
     if (!ctl->imode) {
         ret = vshCommandRun(ctl, ctl->cmd);

-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

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