[libvirt] [PATCH] libxl: unref objects in error paths

24 Feb 2016

libxlMakeNic opens a virConnect object and takes a reference on a
virNetwork object, but doesn't drop the references on all error
paths. Rework the function to follow the standard libvirt pattern
of using a local 'ret' variable to hold the function return value,
performing all cleanup and returning 'ret' at a 'cleanup' label.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

I noticed these leaks while trying to rework the code to use a
common virConnect object. That task has turned into quite an
effort [1], but in the meantime the leaks in the current code
should be plugged.

[1] https://www.redhat.com/archives/libvir-list/2016-February/msg01188.html

 src/libxl/libxl_conf.c | 57 ++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b20df29..763f4c5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1286,7 +1286,11 @@ libxlMakeNic(virDomainDefPtr def,
 {
     bool ioemu_nic = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
     virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
+    char *brname = NULL;
+    virNetworkPtr network = NULL;
+    virConnectPtr conn = NULL;
     virNetDevBandwidthPtr actual_bw;
+    int ret = -1;
 
     /* TODO: Where is mtu stored?
      *
@@ -1312,64 +1316,50 @@ libxlMakeNic(virDomainDefPtr def,
 
     if (l_nic->model) {
         if (VIR_STRDUP(x_nic->model, l_nic->model) < 0)
-            return -1;
+            goto cleanup;
         if (STREQ(l_nic->model, "netfront"))
             x_nic->nictype = LIBXL_NIC_TYPE_VIF;
     }
 
     if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
-        return -1;
+        goto cleanup;
 
     switch (actual_type) {
         case VIR_DOMAIN_NET_TYPE_BRIDGE:
             if (VIR_STRDUP(x_nic->bridge,
                            virDomainNetGetActualBridgeName(l_nic)) < 0)
-                return -1;
+                goto cleanup;
             /* fallthrough */
         case VIR_DOMAIN_NET_TYPE_ETHERNET:
             if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
-                return -1;
+                goto cleanup;
             if (l_nic->nips > 0) {
                 x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address);
                 if (!x_nic->ip)
-                    return -1;
+                    goto cleanup;
             }
             break;
         case VIR_DOMAIN_NET_TYPE_NETWORK:
         {
-            bool fail = false;
-            char *brname = NULL;
-            virNetworkPtr network;
-            virConnectPtr conn;
-
             if (!(conn = virConnectOpen("xen:///system")))
-                return -1;
+                goto cleanup;
 
             if (!(network =
                   virNetworkLookupByName(conn, l_nic->data.network.name))) {
-                virObjectUnref(conn);
-                return -1;
+                goto cleanup;
             }
 
             if (l_nic->nips > 0) {
                 x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address);
                 if (!x_nic->ip)
-                    return -1;
+                    goto cleanup;
             }
 
-            if ((brname = virNetworkGetBridgeName(network))) {
-                if (VIR_STRDUP(x_nic->bridge, brname) < 0)
-                    fail = true;
-            } else {
-                fail = true;
-            }
-
-            VIR_FREE(brname);
+            if (!(brname = virNetworkGetBridgeName(network)))
+                goto cleanup;
 
-            virObjectUnref(network);
-            virObjectUnref(conn);
-            if (fail)
-                return -1;
+            if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+                    goto cleanup;
             break;
         }
         case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
@@ -1385,18 +1375,18 @@ libxlMakeNic(virDomainDefPtr def,
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                     _("unsupported interface type %s"),
                     virDomainNetTypeToString(l_nic->type));
-            return -1;
+            goto cleanup;
     }
 
     if (l_nic->domain_name) {
 #ifdef LIBXL_HAVE_DEVICE_BACKEND_DOMNAME
         if (VIR_STRDUP(x_nic->backend_domname, l_nic->domain_name) < 0)
-            return -1;
+            goto cleanup;
 #else
         virReportError(VIR_ERR_XML_DETAIL, "%s",
                 _("this version of libxenlight does not "
                   "support backend domain name"));
-        return -1;
+        goto cleanup;
 #endif
     }
 
@@ -1438,7 +1428,14 @@ libxlMakeNic(virDomainDefPtr def,
         x_nic->rate_interval_usecs =  50000UL;
     }
 
-    return 0;
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(brname);
+    virObjectUnref(network);
+    virObjectUnref(conn);
+
+    return ret;
 }
 
 static int
-- 
2.6.1

    

Jim Fehlig

Joao Martins

Michal Privoznik

Jim Fehlig

tags

participants (3)