[PATCH 1/8] remote: fix memory leak

[Eric Blake <eblake@xxxxxxxxxx>]

Detected in this valgrind run:
https://bugzilla.redhat.com/show_bug.cgi?id=690734
==13864== 10 bytes in 1 blocks are definitely lost in loss record 10 of 34
==13864==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==13864==    by 0x308587FD91: strdup (in /lib64/libc-2.12.so)
==13864==    by 0x3BB68BA0A3: doRemoteOpen (remote_driver.c:451)
==13864==    by 0x3BB68BCFCF: remoteOpen (remote_driver.c:1111)
==13864==    by 0x3BB689A0FF: do_open (libvirt.c:1290)
==13864==    by 0x3BB689ADD5: virConnectOpenAuth (libvirt.c:1545)
==13864==    by 0x41F659: main (virsh.c:11613)

* src/remote/remote_driver.c (doRemoteClose): Move up.
(remoteOpenSecondaryDriver, remoteOpen): Avoid leak on failure.
---

Original post:
https://www.redhat.com/archives/libvir-list/2011-March/msg01196.html

 src/remote/remote_driver.c |  120 +++++++++++++++++++++++---------------------
 1 files changed, 62 insertions(+), 58 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b05bbcb..4c7df17 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -998,6 +998,64 @@ retry:
     goto cleanup;
 }

+static int
+doRemoteClose (virConnectPtr conn, struct private_data *priv)
+{
+    if (priv->eventFlushTimer >= 0) {
+        /* Remove timeout */
+        virEventRemoveTimeout(priv->eventFlushTimer);
+        /* Remove handle for remote events */
+        virEventRemoveHandle(priv->watch);
+        priv->watch = -1;
+    }
+
+    if (call (conn, priv, 0, REMOTE_PROC_CLOSE,
+              (xdrproc_t) xdr_void, (char *) NULL,
+              (xdrproc_t) xdr_void, (char *) NULL) == -1)
+        return -1;
+
+    /* Close socket. */
+    if (priv->uses_tls && priv->session) {
+        gnutls_bye (priv->session, GNUTLS_SHUT_RDWR);
+        gnutls_deinit (priv->session);
+    }
+#if HAVE_SASL
+    if (priv->saslconn)
+        sasl_dispose (&priv->saslconn);
+#endif
+    VIR_FORCE_CLOSE(priv->sock);
+    VIR_FORCE_CLOSE(priv->errfd);
+
+#ifndef WIN32
+    if (priv->pid > 0) {
+        pid_t reap;
+        do {
+retry:
+            reap = waitpid(priv->pid, NULL, 0);
+            if (reap == -1 && errno == EINTR)
+                goto retry;
+        } while (reap != -1 && reap != priv->pid);
+    }
+#endif
+    VIR_FORCE_CLOSE(priv->wakeupReadFD);
+    VIR_FORCE_CLOSE(priv->wakeupSendFD);
+
+
+    /* Free hostname copy */
+    VIR_FREE(priv->hostname);
+
+    /* See comment for remoteType. */
+    VIR_FREE(priv->type);
+
+    /* Free callback list */
+    virDomainEventCallbackListFree(priv->callbackList);
+
+    /* Free queued events */
+    virDomainEventQueueFree(priv->domainEvents);
+
+    return 0;
+}
+
 static struct private_data *
 remoteAllocPrivateData(void)
 {
@@ -1039,7 +1097,9 @@ remoteOpenSecondaryDriver(virConnectPtr conn,

     ret = doRemoteOpen(conn, *priv, auth, rflags);
     if (ret != VIR_DRV_OPEN_SUCCESS) {
+        doRemoteClose(conn, *priv);
         remoteDriverUnlock(*priv);
+        virMutexDestroy(&(*priv)->lock);
         VIR_FREE(*priv);
     } else {
         (*priv)->localUses = 1;
@@ -1111,7 +1171,9 @@ remoteOpen (virConnectPtr conn,
     ret = doRemoteOpen(conn, priv, auth, rflags);
     if (ret != VIR_DRV_OPEN_SUCCESS) {
         conn->privateData = NULL;
+        doRemoteClose(conn, priv);
         remoteDriverUnlock(priv);
+        virMutexDestroy(&priv->lock);
         VIR_FREE(priv);
     } else {
         conn->privateData = priv;
@@ -1538,64 +1600,6 @@ verify_certificate (virConnectPtr conn ATTRIBUTE_UNUSED,


 static int
-doRemoteClose (virConnectPtr conn, struct private_data *priv)
-{
-    if (priv->eventFlushTimer >= 0) {
-        /* Remove timeout */
-        virEventRemoveTimeout(priv->eventFlushTimer);
-        /* Remove handle for remote events */
-        virEventRemoveHandle(priv->watch);
-        priv->watch = -1;
-    }
-
-    if (call (conn, priv, 0, REMOTE_PROC_CLOSE,
-              (xdrproc_t) xdr_void, (char *) NULL,
-              (xdrproc_t) xdr_void, (char *) NULL) == -1)
-        return -1;
-
-    /* Close socket. */
-    if (priv->uses_tls && priv->session) {
-        gnutls_bye (priv->session, GNUTLS_SHUT_RDWR);
-        gnutls_deinit (priv->session);
-    }
-#if HAVE_SASL
-    if (priv->saslconn)
-        sasl_dispose (&priv->saslconn);
-#endif
-    VIR_FORCE_CLOSE(priv->sock);
-    VIR_FORCE_CLOSE(priv->errfd);
-
-#ifndef WIN32
-    if (priv->pid > 0) {
-        pid_t reap;
-        do {
-retry:
-            reap = waitpid(priv->pid, NULL, 0);
-            if (reap == -1 && errno == EINTR)
-                goto retry;
-        } while (reap != -1 && reap != priv->pid);
-    }
-#endif
-    VIR_FORCE_CLOSE(priv->wakeupReadFD);
-    VIR_FORCE_CLOSE(priv->wakeupSendFD);
-
-
-    /* Free hostname copy */
-    VIR_FREE(priv->hostname);
-
-    /* See comment for remoteType. */
-    VIR_FREE(priv->type);
-
-    /* Free callback list */
-    virDomainEventCallbackListFree(priv->callbackList);
-
-    /* Free queued events */
-    virDomainEventQueueFree(priv->domainEvents);
-
-    return 0;
-}
-
-static int
 remoteClose (virConnectPtr conn)
 {
     int ret = 0;
-- 
1.7.4

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