lib: Introduce and use g_autoptr() for virInterfaceDef

There are a lot of places where we call virInterfaceDefFree()
explicitly. We can define autoptr cleanup macro and annotate
declarations with g_autoptr() and remove plenty of those explicit
free calls.

This also fixes a memory leak in udevInterfaceGetXMLDesc() which
called virInterfaceDefFree() only in successful path.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>

diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index b45dc3737934631bbb9888909a8eadabf199edbb..f2b3804bec3ff75ed97a3b1f6065e72e9395e4cd 100644 (file)
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -679,7 +679,7 @@ static virInterfaceDef *
 virInterfaceDefParseXML(xmlXPathContextPtr ctxt,
                         int parentIfType)
 {
-    virInterfaceDef *def;
+    g_autoptr(virInterfaceDef) def = NULL;
     int type;
     char *tmp;
     VIR_XPATH_NODE_AUTORESTORE(ctxt)
@@ -716,28 +716,28 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt,
         virReportError(VIR_ERR_XML_ERROR,
                        _("interface has unsupported type '%s'"),
                        virInterfaceTypeToString(type));
-        goto error;
+        return NULL;
     }
     def->type = type;
 
     if (virInterfaceDefParseName(def, ctxt) < 0)
-       goto error;
+       return NULL;
 
     if (parentIfType == VIR_INTERFACE_TYPE_LAST) {
         /* only recognize these in toplevel bond interfaces */
         if (virInterfaceDefParseStartMode(def, ctxt) < 0)
-            goto error;
+            return NULL;
         if (virInterfaceDefParseMtu(def, ctxt) < 0)
-            goto error;
+            return NULL;
         if (virInterfaceDefParseIfAdressing(def, ctxt) < 0)
-            goto error;
+            return NULL;
     }
 
     if (type != VIR_INTERFACE_TYPE_BRIDGE) {
         /* link status makes no sense for a bridge */
         lnk = virXPathNode("./link", ctxt);
         if (lnk && virInterfaceLinkParseXML(lnk, &def->lnk) < 0)
-            goto error;
+            return NULL;
     }
 
     switch (type) {
@@ -751,11 +751,11 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt,
             if (!(bridge = virXPathNode("./bridge[1]", ctxt))) {
                 virReportError(VIR_ERR_XML_ERROR,
                                "%s", _("bridge interface misses the bridge element"));
-                goto error;
+                return NULL;
             }
             ctxt->node = bridge;
             if (virInterfaceDefParseBridge(def, ctxt) < 0)
-                goto error;
+                return NULL;
             break;
         }
         case VIR_INTERFACE_TYPE_BOND: {
@@ -764,11 +764,11 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt,
             if (!(bond = virXPathNode("./bond[1]", ctxt))) {
                 virReportError(VIR_ERR_XML_ERROR,
                                "%s", _("bond interface misses the bond element"));
-                goto error;
+                return NULL;
             }
             ctxt->node = bond;
             if (virInterfaceDefParseBond(def, ctxt)  < 0)
-                goto error;
+                return NULL;
             break;
         }
         case VIR_INTERFACE_TYPE_VLAN: {
@@ -777,21 +777,17 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt,
             if (!(vlan = virXPathNode("./vlan[1]", ctxt))) {
                 virReportError(VIR_ERR_XML_ERROR,
                                "%s", _("vlan interface misses the vlan element"));
-                goto error;
+                return NULL;
             }
             ctxt->node = vlan;
             if (virInterfaceDefParseVlan(def, ctxt)  < 0)
-                goto error;
+                return NULL;
             break;
         }
 
     }
 
-    return def;
-
- error:
-    virInterfaceDefFree(def);
-    return NULL;
+    return g_steal_pointer(&def);
 }
 
 

diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index ea92e0fb31cc082132648b5b79e420677f00e045..510d83b2bf0744aa4d6ec4f4c6eef0f65bdfe4bb 100644 (file)
--- a/src/conf/interface_conf.h
+++ b/src/conf/interface_conf.h
@@ -153,6 +153,7 @@ struct _virInterfaceDef {
 
 void
 virInterfaceDefFree(virInterfaceDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virInterfaceDef, virInterfaceDefFree);
 
 virInterfaceDef *
 virInterfaceDefParseString(const char *xmlStr,

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 9439bb3d0bcfedb9d9f3e7d5f5eded4c044583a7..fa7a21c054a7c33afeaea4c06f42de829d5d6ac5 100644 (file)
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -362,7 +362,7 @@ virInterfaceObjListCloneCb(void *payload,
     virInterfaceObj *srcObj = payload;
     struct _virInterfaceObjListCloneData *data = opaque;
     char *xml = NULL;
-    virInterfaceDef *backup = NULL;
+    g_autoptr(virInterfaceDef) backup = NULL;
     virInterfaceObj *obj;
 
     if (data->error)
@@ -379,6 +379,7 @@ virInterfaceObjListCloneCb(void *payload,
 
     if (!(obj = virInterfaceObjListAssignDef(data->dest, backup)))
         goto error;
+    backup = NULL;
     virInterfaceObjEndAPI(&obj);
 
     virObjectUnlock(srcObj);
@@ -387,7 +388,6 @@ virInterfaceObjListCloneCb(void *payload,
  error:
     data->error = true;
     VIR_FREE(xml);
-    virInterfaceDefFree(backup);
     virObjectUnlock(srcObj);
     return 0;
 }

diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
index 78fd4f9bc773b4379624adb25f311acd09d2a075..146a7039537d67f7b8330763192d162e25c7e1ca 100644 (file)
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -387,7 +387,7 @@ static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn,
     }
 
     for (i = 0; i < count; i++) {
-        virInterfaceDef *def;
+        g_autoptr(virInterfaceDef) def = NULL;
         struct netcf_if *iface;
 
         iface = ncf_lookup_by_name(driver->netcf, names[i]);
@@ -416,11 +416,8 @@ static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn,
         }
         ncf_if_free(iface);
 
-        if (!filter(conn, def)) {
-            virInterfaceDefFree(def);
+        if (!filter(conn, def))
             continue;
-        }
-        virInterfaceDefFree(def);
 
         want++;
     }
@@ -481,7 +478,7 @@ static int netcfConnectListInterfacesImpl(virConnectPtr conn,
     }
 
     for (i = 0; i < count && want < nnames; i++) {
-        virInterfaceDef *def;
+        g_autoptr(virInterfaceDef) def = NULL;
         struct netcf_if *iface;
 
         iface = ncf_lookup_by_name(driver->netcf, allnames[i]);
@@ -510,11 +507,8 @@ static int netcfConnectListInterfacesImpl(virConnectPtr conn,
         }
         ncf_if_free(iface);
 
-        if (!filter(conn, def)) {
-            virInterfaceDefFree(def);
+        if (!filter(conn, def))
             continue;
-        }
-        virInterfaceDefFree(def);
 
         names[want++] = g_steal_pointer(&allnames[i]);
     }
@@ -665,7 +659,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
         tmp_iface_objs = g_new0(virInterfacePtr, count + 1);
 
     for (i = 0; i < count; i++) {
-        virInterfaceDef *def;
+        g_autoptr(virInterfaceDef) def = NULL;
 
         iface = ncf_lookup_by_name(driver->netcf, names[i]);
         if (!iface) {
@@ -693,20 +687,17 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
         if (!virConnectListAllInterfacesCheckACL(conn, def)) {
             ncf_if_free(iface);
             iface = NULL;
-            virInterfaceDefFree(def);
             continue;
         }
 
         if (ifaces) {
-            if (!(iface_obj = virGetInterface(conn, def->name, def->mac))) {
-                virInterfaceDefFree(def);
+            if (!(iface_obj = virGetInterface(conn, def->name, def->mac)))
                 goto cleanup;
-            }
+
             tmp_iface_objs[niface_objs] = iface_obj;
         }
         niface_objs++;
 
-        virInterfaceDefFree(def);
         ncf_if_free(iface);
         iface = NULL;
     }
@@ -743,7 +734,7 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
 {
     struct netcf_if *iface;
     virInterfacePtr ret = NULL;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
 
     virObjectLock(driver);
     iface = ncf_lookup_by_name(driver->netcf, name);
@@ -772,7 +763,6 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
 
  cleanup:
     ncf_if_free(iface);
-    virInterfaceDefFree(def);
     virObjectUnlock(driver);
     return ret;
 }
@@ -783,7 +773,7 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn,
     struct netcf_if *iface;
     int niface;
     virInterfacePtr ret = NULL;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
 
     virObjectLock(driver);
     niface = ncf_lookup_by_mac_string(driver->netcf, macstr, 1, &iface);
@@ -820,7 +810,6 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn,
 
  cleanup:
     ncf_if_free(iface);
-    virInterfaceDefFree(def);
     virObjectUnlock(driver);
     return ret;
 }
@@ -830,7 +819,7 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
 {
     struct netcf_if *iface = NULL;
     char *xmlstr = NULL;
-    virInterfaceDef *ifacedef = NULL;
+    g_autoptr(virInterfaceDef) ifacedef = NULL;
     char *ret = NULL;
     bool active;
 
@@ -880,7 +869,6 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
  cleanup:
     ncf_if_free(iface);
     VIR_FREE(xmlstr);
-    virInterfaceDefFree(ifacedef);
     virObjectUnlock(driver);
     return ret;
 }
@@ -891,7 +879,7 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
 {
     struct netcf_if *iface = NULL;
     char *xmlstr = NULL;
-    virInterfaceDef *ifacedef = NULL;
+    g_autoptr(virInterfaceDef) ifacedef = NULL;
     virInterfacePtr ret = NULL;
 
     virCheckFlags(VIR_INTERFACE_DEFINE_VALIDATE, NULL);
@@ -929,7 +917,6 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
  cleanup:
     ncf_if_free(iface);
     VIR_FREE(xmlstr);
-    virInterfaceDefFree(ifacedef);
     virObjectUnlock(driver);
     return ret;
 }
@@ -937,7 +924,7 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
 static int netcfInterfaceUndefine(virInterfacePtr ifinfo)
 {
     struct netcf_if *iface = NULL;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
 
     virObjectLock(driver);
@@ -968,7 +955,6 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo)
 
  cleanup:
     ncf_if_free(iface);
-    virInterfaceDefFree(def);
     virObjectUnlock(driver);
     return ret;
 }
@@ -977,7 +963,7 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
                                 unsigned int flags)
 {
     struct netcf_if *iface = NULL;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
     bool active;
 
@@ -1020,7 +1006,6 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
 
  cleanup:
     ncf_if_free(iface);
-    virInterfaceDefFree(def);
     virObjectUnlock(driver);
     return ret;
 }
@@ -1029,7 +1014,7 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
                                  unsigned int flags)
 {
     struct netcf_if *iface = NULL;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
     bool active;
 
@@ -1072,7 +1057,6 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
 
  cleanup:
     ncf_if_free(iface);
-    virInterfaceDefFree(def);
     virObjectUnlock(driver);
     return ret;
 }
@@ -1080,7 +1064,7 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
 static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
 {
     struct netcf_if *iface = NULL;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
     bool active;
 
@@ -1105,7 +1089,6 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
 
  cleanup:
     ncf_if_free(iface);
-    virInterfaceDefFree(def);
     virObjectUnlock(driver);
     return ret;
 }

diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
index 0217f166073e19663a2f43d3ffa08a132a1563ff..8c417714e59a1bb958317b322262517abc4b40a5 100644 (file)
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -165,7 +165,7 @@ udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus status,
     udev_list_entry_foreach(dev_entry, devices) {
         struct udev_device *dev;
         const char *path;
-        virInterfaceDef *def;
+        g_autoptr(virInterfaceDef) def = NULL;
 
         path = udev_list_entry_get_name(dev_entry);
         dev = udev_device_new_from_syspath(udev, path);
@@ -174,7 +174,6 @@ udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus status,
         if (filter(conn, def))
             count++;
         udev_device_unref(dev);
-        virInterfaceDefFree(def);
     }
 
  cleanup:
@@ -218,7 +217,7 @@ udevListInterfacesByStatus(virConnectPtr conn,
     udev_list_entry_foreach(dev_entry, devices) {
         struct udev_device *dev;
         const char *path;
-        virInterfaceDef *def;
+        g_autoptr(virInterfaceDef) def = NULL;
 
         /* Ensure we won't exceed the size of our array */
         if (count > names_len)
@@ -233,7 +232,6 @@ udevListInterfacesByStatus(virConnectPtr conn,
             count++;
         }
         udev_device_unref(dev);
-        virInterfaceDefFree(def);
     }
 
     udev_enumerate_unref(enumerate);
@@ -355,7 +353,7 @@ udevConnectListAllInterfaces(virConnectPtr conn,
         const char *path;
         const char *name;
         const char *macaddr;
-        virInterfaceDef *def;
+        g_autoptr(virInterfaceDef) def = NULL;
 
         path = udev_list_entry_get_name(dev_entry);
         dev = udev_device_new_from_syspath(udev, path);
@@ -366,10 +364,8 @@ udevConnectListAllInterfaces(virConnectPtr conn,
         def = udevGetMinimalDefForDevice(dev);
         if (!virConnectListAllInterfacesCheckACL(conn, def)) {
             udev_device_unref(dev);
-            virInterfaceDefFree(def);
             continue;
         }
-        virInterfaceDefFree(def);
 
         /* Filter the results */
         if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) &&
@@ -413,7 +409,7 @@ udevInterfaceLookupByName(virConnectPtr conn, const char *name)
     struct udev *udev = udev_ref(driver->udev);
     struct udev_device *dev;
     virInterfacePtr ret = NULL;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
 
     /* get a device reference based on the device name */
     dev = udev_device_new_from_subsystem_sysname(udev, "net", name);
@@ -435,7 +431,6 @@ udevInterfaceLookupByName(virConnectPtr conn, const char *name)
 
  cleanup:
     udev_unref(udev);
-    virInterfaceDefFree(def);
 
     return ret;
 }
@@ -447,7 +442,7 @@ udevInterfaceLookupByMACString(virConnectPtr conn, const char *macstr)
     struct udev_enumerate *enumerate = NULL;
     struct udev_list_entry *dev_entry;
     struct udev_device *dev;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
     virInterfacePtr ret = NULL;
 
     enumerate = udevGetDevices(udev, VIR_UDEV_IFACE_ALL);
@@ -499,7 +494,6 @@ udevInterfaceLookupByMACString(virConnectPtr conn, const char *macstr)
     if (enumerate)
         udev_enumerate_unref(enumerate);
     udev_unref(udev);
-    virInterfaceDefFree(def);
 
     return ret;
 }
@@ -945,7 +939,7 @@ static virInterfaceDef * ATTRIBUTE_NONNULL(1)
 udevGetIfaceDef(struct udev *udev, const char *name)
 {
     struct udev_device *dev = NULL;
-    virInterfaceDef *ifacedef;
+    g_autoptr(virInterfaceDef) ifacedef = NULL;
     unsigned int mtu;
     const char *mtu_str;
     char *vlan_parent_dev = NULL;
@@ -1038,13 +1032,11 @@ udevGetIfaceDef(struct udev *udev, const char *name)
 
     udev_device_unref(dev);
 
-    return ifacedef;
+    return g_steal_pointer(&ifacedef);
 
  error:
     udev_device_unref(dev);
 
-    virInterfaceDefFree(ifacedef);
-
     return NULL;
 }
 
@@ -1053,7 +1045,7 @@ udevInterfaceGetXMLDesc(virInterfacePtr ifinfo,
                         unsigned int flags)
 {
     struct udev *udev = udev_ref(driver->udev);
-    virInterfaceDef *ifacedef;
+    g_autoptr(virInterfaceDef) ifacedef = NULL;
     char *xmlstr = NULL;
 
     virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
@@ -1071,8 +1063,6 @@ udevInterfaceGetXMLDesc(virInterfacePtr ifinfo,
 
     xmlstr = virInterfaceDefFormat(ifacedef);
 
-    virInterfaceDefFree(ifacedef);
-
  cleanup:
     /* decrement our udev ptr */
     udev_unref(udev);
@@ -1085,7 +1075,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
 {
     struct udev *udev = udev_ref(driver->udev);
     struct udev_device *dev;
-    virInterfaceDef *def = NULL;
+    g_autoptr(virInterfaceDef) def = NULL;
     int status = -1;
 
     dev = udev_device_new_from_subsystem_sysname(udev, "net",
@@ -1110,7 +1100,6 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
 
  cleanup:
     udev_unref(udev);
-    virInterfaceDefFree(def);
 
     return status;
 }

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 13d07e570e8b667ae31e2c7780035d35a3c5142c..ea474d55ac62c740c5822bba7be5bd53316feaae 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1096,7 +1096,7 @@ testParseNetworks(testDriver *privconn,
         return -1;
 
     for (i = 0; i < num; i++) {
-        virNetworkDef *def;
+        g_autoptr(virNetworkDef) def = NULL;
         xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "network");
         if (!node)
             return -1;
@@ -1105,10 +1105,9 @@ testParseNetworks(testDriver *privconn,
         if (!def)
             return -1;
 
-        if (!(obj = virNetworkObjAssignDef(privconn->networks, def, 0))) {
-            virNetworkDefFree(def);
+        if (!(obj = virNetworkObjAssignDef(privconn->networks, def, 0)))
             return -1;
-        }
+        def = NULL;
 
         virNetworkObjSetActive(obj, true);
         virNetworkObjEndAPI(&obj);
@@ -1133,7 +1132,7 @@ testParseInterfaces(testDriver *privconn,
         return -1;
 
     for (i = 0; i < num; i++) {
-        virInterfaceDef *def;
+        g_autoptr(virInterfaceDef) def = NULL;
         xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file,
                                                    "interface");
         if (!node)
@@ -1143,10 +1142,9 @@ testParseInterfaces(testDriver *privconn,
         if (!def)
             return -1;
 
-        if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces, def))) {
-            virInterfaceDefFree(def);
+        if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces, def)))
             return -1;
-        }
+        def = NULL;
 
         virInterfaceObjSetActive(obj, true);
         virInterfaceObjEndAPI(&obj);
@@ -6195,7 +6193,7 @@ testInterfaceDefineXML(virConnectPtr conn,
                        unsigned int flags)
 {
     testDriver *privconn = conn->privateData;
-    virInterfaceDef *def;
+    g_autoptr(virInterfaceDef) def = NULL;
     virInterfaceObj *obj = NULL;
     virInterfaceDef *objdef;
     virInterfacePtr ret = NULL;
@@ -6214,7 +6212,6 @@ testInterfaceDefineXML(virConnectPtr conn,
     ret = virGetInterface(conn, objdef->name, objdef->mac);
 
  cleanup:
-    virInterfaceDefFree(def);
     virInterfaceObjEndAPI(&obj);
     virObjectUnlock(privconn);
     return ret;

diff --git a/tests/interfacexml2xmltest.c b/tests/interfacexml2xmltest.c
index 8235e701a919810255488de822f3529f3e4b8381..15fab8810798fac16a48745586eadaadfbc64848 100644 (file)
--- a/tests/interfacexml2xmltest.c
+++ b/tests/interfacexml2xmltest.c
@@ -18,28 +18,23 @@ testCompareXMLToXMLFiles(const char *xml)
 {
     g_autofree char *xmlData = NULL;
     g_autofree char *actual = NULL;
-    int ret = -1;
-    virInterfaceDef *dev = NULL;
+    g_autoptr(virInterfaceDef) dev = NULL;
 
     if (virTestLoadFile(xml, &xmlData) < 0)
-        goto fail;
+        return -1;
 
     if (!(dev = virInterfaceDefParseString(xmlData, 0)))
-        goto fail;
+        return -1;
 
     if (!(actual = virInterfaceDefFormat(dev)))
-        goto fail;
+        return -1;
 
     if (STRNEQ(xmlData, actual)) {
         virTestDifferenceFull(stderr, xmlData, xml, actual, NULL);
-        goto fail;
+        return -1;
     }
 
-    ret = 0;
-
- fail:
-    virInterfaceDefFree(dev);
-    return ret;
+    return 0;
 }
 
 static int