From e866e302f86313ad890db55e2b7890236f1fd47e Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Wed, 21 May 2008 20:53:30 +0000
Subject: [PATCH] Fixed couple of memory leaks wrt to virXXXDestroy APIs, and
 clarify docs to match reality

---
 ChangeLog             | 12 ++++++++++++
 qemud/remote.c        |  6 ++++--
 src/hash.c            |  3 +++
 src/libvirt.c         | 14 ++++++--------
 src/remote_internal.c |  4 ++++
 src/virsh.c           |  6 +++---
 6 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0fd68a20e6..d4e52be70c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+Wed May 21 16:24:29 EST 2008 Daniel P. Berrange <berrange@redhat.com>
+
+	Fix up misc memory leaks / incorrect docs (Cole Robinson)
+	* qemud/remote.c: Free the virDomainPtr object in the
+	virDomainDestroy handler to avoid leak
+	* src/hash.c: Added more ref count debug
+	* src/libvirt.c: Clarified docs on virXXXDestroy to
+	match reality
+	* src/remote_internal.c: Free virDomain/Network objects
+	after dispatching RPC error
+	* src/virsh.c: Added some more virDomainFree calls
+
 Wed May 21 15:42:29 EST 2008 Daniel P. Berrange <berrange@redhat.com>
 
 	* python/generator.py: Don't free the underlying virDomainPtr
diff --git a/qemud/remote.c b/qemud/remote.c
index a97641ae19..725152e5ff 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -940,9 +940,11 @@ remoteDispatchDomainDestroy (struct qemud_server *server ATTRIBUTE_UNUSED,
         return -2;
     }
 
-    if (virDomainDestroy (dom) == -1)
+    if (virDomainDestroy (dom) == -1) {
+        virDomainFree(dom);
         return -1;
-    /* No need to free dom - destroy does it for us */
+    }
+    virDomainFree(dom);
     return 0;
 }
 
diff --git a/src/hash.c b/src/hash.c
index 79421aad43..01def44d91 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -842,6 +842,9 @@ __virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid)
             goto error;
         }
         conn->refs++;
+        DEBUG("New hash entry %p", ret);
+    } else {
+        DEBUG("Existing hash entry %p: refs now %d", ret, ret->refs+1);
     }
     ret->refs++;
     pthread_mutex_unlock(&conn->lock);
diff --git a/src/libvirt.c b/src/libvirt.c
index 97f6bc37be..9f6df8ebc3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1390,10 +1390,9 @@ virDomainLookupByName(virConnectPtr conn, const char *name)
  * @domain: a domain object
  *
  * Destroy the domain object. The running instance is shutdown if not down
- * already and all resources used by it are given back to the hypervisor.
- * The data structure is freed and should not be used thereafter if the
- * call does not return an error.
- * This function may requires privileged access
+ * already and all resources used by it are given back to the hypervisor. This
+ * does not free the associated virDomainPtr object.
+ * This function may require privileged access
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
@@ -3502,10 +3501,9 @@ virNetworkCreate(virNetworkPtr network)
  * @network: a network object
  *
  * Destroy the network object. The running instance is shutdown if not down
- * already and all resources used by it are given back to the hypervisor.
- * The data structure is freed and should not be used thereafter if the
- * call does not return an error.
- * This function may requires privileged access
+ * already and all resources used by it are given back to the hypervisor. This
+ * does not free the associated virNetworkPtr object.
+ * This function may require privileged access
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 51e8eb77e9..c17b8910fc 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -4606,6 +4606,10 @@ server_error (virConnectPtr conn, remote_error *err)
                      err->str3 ? *err->str3 : NULL,
                      err->int1, err->int2,
                      "%s", err->message ? *err->message : NULL);
+    if (dom)
+        virDomainFree(dom);
+    if (net)
+        virNetworkFree(net);
 }
 
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/virsh.c b/src/virsh.c
index 45af630800..234fc363e7 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -1468,9 +1468,9 @@ cmdDestroy(vshControl * ctl, vshCmd * cmd)
     } else {
         vshError(ctl, FALSE, _("Failed to destroy domain %s"), name);
         ret = FALSE;
-        virDomainFree(dom);
     }
 
+    virDomainFree(dom);
     return ret;
 }
 
@@ -2433,9 +2433,9 @@ cmdNetworkDestroy(vshControl * ctl, vshCmd * cmd)
     } else {
         vshError(ctl, FALSE, _("Failed to destroy network %s"), name);
         ret = FALSE;
-        virNetworkFree(network);
     }
 
+    virNetworkFree(network);
     return ret;
 }
 
@@ -3161,9 +3161,9 @@ cmdPoolDestroy(vshControl * ctl, vshCmd * cmd)
     } else {
         vshError(ctl, FALSE, _("Failed to destroy pool %s"), name);
         ret = FALSE;
-        virStoragePoolFree(pool);
     }
 
+    virStoragePoolFree(pool);
     return ret;
 }
 
-- 
GitLab