conf: take advantage of VIR_AUTOPTR for virNetworkPortDefPtr

define a VIR_DEFINE_AUTOPTR_FUNC() to autofree virNetworkPortDefs, and
convert all uses of virNetworkPortDefPtr that are appropriate to use
it.

This coincidentally fixes multiple potential memory leaks (in failure
cases) in networkPortCreateXML()

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

src/conf/domain_conf.c

@@ -30671,7 +30671,7 @@ virNetworkPortDefPtr
 virDomainNetDefToNetworkPort(virDomainDefPtr dom,
                              virDomainNetDefPtr iface)
 {
-    virNetworkPortDefPtr port;
+    VIR_AUTOPTR(virNetworkPortDef) port = NULL;
 
     if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -30686,34 +30686,30 @@ virDomainNetDefToNetworkPort(virDomainDefPtr dom,
     if (virUUIDGenerate(port->uuid) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("Failed to generate UUID"));
-        goto error;
+        return NULL;
     }
 
     memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN);
     if (VIR_STRDUP(port->ownername, dom->name) < 0)
-        goto error;
+        return NULL;
 
     if (VIR_STRDUP(port->group, iface->data.network.portgroup) < 0)
-        goto error;
+        return NULL;
 
     memcpy(&port->mac, &iface->mac, VIR_MAC_BUFLEN);
 
     if (virNetDevVPortProfileCopy(&port->virtPortProfile, iface->virtPortProfile) < 0)
-        goto error;
+        return NULL;
 
     if (virNetDevBandwidthCopy(&port->bandwidth, iface->bandwidth) < 0)
-        goto error;
+        return NULL;
 
     if (virNetDevVlanCopy(&port->vlan, &iface->vlan) < 0)
-        goto error;
+        return NULL;
 
     port->trustGuestRxFilters = iface->trustGuestRxFilters;
 
-    return port;
-
- error:
-    virNetworkPortDefFree(port);
-    return NULL;
+    VIR_RETURN_PTR(port);
 }
 
 int
@@ -30834,7 +30830,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
                                    virDomainNetDefPtr iface)
 {
     virDomainActualNetDefPtr actual;
-    virNetworkPortDefPtr port;
+    VIR_AUTOPTR(virNetworkPortDef) port = NULL;
 
     if (!iface->data.network.actual) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -30859,15 +30855,15 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
     if (virUUIDGenerate(port->uuid) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("Failed to generate UUID"));
-        goto error;
+        return NULL;
     }
 
     memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN);
     if (VIR_STRDUP(port->ownername, dom->name) < 0)
-        goto error;
+        return NULL;
 
     if (VIR_STRDUP(port->group, iface->data.network.portgroup) < 0)
-        goto error;
+        return NULL;
 
     memcpy(&port->mac, &iface->mac, VIR_MAC_BUFLEN);
 
@@ -30876,7 +30872,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
         port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_NETWORK;
         if (VIR_STRDUP(port->plug.bridge.brname,
                        actual->data.bridge.brname) < 0)
-            goto error;
+            return NULL;
         port->plug.bridge.macTableManager = actual->data.bridge.macTableManager;
         break;
 
@@ -30884,7 +30880,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
         port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE;
         if (VIR_STRDUP(port->plug.bridge.brname,
                        actual->data.bridge.brname) < 0)
-            goto error;
+            return NULL;
         port->plug.bridge.macTableManager = actual->data.bridge.macTableManager;
         break;
 
@@ -30892,7 +30888,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
         port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_DIRECT;
         if (VIR_STRDUP(port->plug.direct.linkdev,
                        actual->data.direct.linkdev) < 0)
-            goto error;
+            return NULL;
         port->plug.direct.mode = actual->data.direct.mode;
         break;
 
@@ -30903,7 +30899,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Actual interface '%s' hostdev was not a PCI device"),
                            iface->ifname);
-            goto error;
+            return NULL;
         }
         port->plug.hostdevpci.managed = virTristateBoolFromBool(actual->data.hostdev.def.managed);
         port->plug.hostdevpci.addr = actual->data.hostdev.def.source.subsys.u.pci.addr;
@@ -30929,7 +30925,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
         default:
             virReportEnumRangeError(virDomainHostdevSubsysPCIBackendType,
                                     actual->data.hostdev.def.source.subsys.u.pci.backend);
-            goto error;
+            return NULL;
         }
 
         break;
@@ -30945,31 +30941,27 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("Unexpected network port type %s"),
                        virDomainNetTypeToString(virDomainNetGetActualType(iface)));
-        goto error;
+        return NULL;
 
     case VIR_DOMAIN_NET_TYPE_LAST:
     default:
         virReportEnumRangeError(virNetworkPortPlugType, port->plugtype);
-        goto error;
+        return NULL;
     }
 
     if (virNetDevVPortProfileCopy(&port->virtPortProfile, actual->virtPortProfile) < 0)
-        goto error;
+        return NULL;
 
     if (virNetDevBandwidthCopy(&port->bandwidth, actual->bandwidth) < 0)
-        goto error;
+        return NULL;
 
     if (virNetDevVlanCopy(&port->vlan, &actual->vlan) < 0)
-        goto error;
+        return NULL;
 
     port->class_id = actual->class_id;
     port->trustGuestRxFilters = actual->trustGuestRxFilters;
 
-    return port;
-
- error:
-    virNetworkPortDefFree(port);
-    return NULL;
+    VIR_RETURN_PTR(port);
 }
 
 
@@ -30979,60 +30971,47 @@ virDomainNetCreatePort(virConnectPtr conn,
                        virDomainNetDefPtr iface,
                        unsigned int flags)
 {
-    virNetworkPtr net = NULL;
-    int ret = -1;
-    virNetworkPortDefPtr portdef = NULL;
-    virNetworkPortPtr port = NULL;
-    char *portxml = NULL;
-    virErrorPtr saved;
+    virErrorPtr save_err;
+    VIR_AUTOUNREF(virNetworkPtr) net = NULL;
+    VIR_AUTOPTR(virNetworkPortDef) portdef = NULL;
+    VIR_AUTOUNREF(virNetworkPortPtr) port = NULL;
+    VIR_AUTOFREE(char *) portxml = NULL;
 
     if (!(net = virNetworkLookupByName(conn, iface->data.network.name)))
         return -1;
 
     if (flags & VIR_NETWORK_PORT_CREATE_RECLAIM) {
         if (!(portdef = virDomainNetDefActualToNetworkPort(dom, iface)))
-            goto cleanup;
+            return -1;
     } else {
         if (!(portdef = virDomainNetDefToNetworkPort(dom, iface)))
-            goto cleanup;
+            return -1;
     }
 
     if (!(portxml = virNetworkPortDefFormat(portdef)))
-        goto cleanup;
+        return -1;
 
+    /* prepare to re-use portdef */
     virNetworkPortDefFree(portdef);
     portdef = NULL;
 
     if (!(port = virNetworkPortCreateXML(net, portxml, flags)))
-        goto cleanup;
+        return -1;
 
+    /* prepare to re-use portxml */
     VIR_FREE(portxml);
 
-    if (!(portxml = virNetworkPortGetXMLDesc(port, 0)))
-        goto deleteport;
-
-    if (!(portdef = virNetworkPortDefParseString(portxml)))
-        goto deleteport;
-
-    if (virDomainNetDefActualFromNetworkPort(iface, portdef) < 0)
-        goto deleteport;
+    if (!(portxml = virNetworkPortGetXMLDesc(port, 0)) ||
+        !(portdef = virNetworkPortDefParseString(portxml)) ||
+        virDomainNetDefActualFromNetworkPort(iface, portdef) < 0) {
+        virErrorPreserveLast(&save_err);
+        virNetworkPortDelete(port, 0);
+        virErrorRestore(&save_err);
+        return -1;
+    }
 
     virNetworkPortGetUUID(port, iface->data.network.portid);
-
-    ret = 0;
- cleanup:
-    virNetworkPortDefFree(portdef);
-    VIR_FREE(portxml);
-    virObjectUnref(port);
-    virObjectUnref(net);
-    return ret;
-
- deleteport:
-    saved = virSaveLastError();
-    virNetworkPortDelete(port, 0);
-    virSetError(saved);
-    virFreeError(saved);
-    goto cleanup;
+    return 0;
 }
 
 int

src/conf/virnetworkobj.c

@@ -1887,7 +1887,7 @@ virNetworkObjLoadAllPorts(virNetworkObjPtr net,
     int ret = -1;
     int rc;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
-    virNetworkPortDefPtr portdef = NULL;
+    VIR_AUTOPTR(virNetworkPortDef) portdef = NULL;
 
     if (!(dir = virNetworkObjGetPortStatusDir(net, stateDir)))
         goto cleanup;
@@ -1925,6 +1925,5 @@ virNetworkObjLoadAllPorts(virNetworkObjPtr net,
     ret = 0;
  cleanup:
     VIR_DIR_CLOSE(dh);
-    virNetworkPortDefFree(portdef);
     return ret;
 }

src/conf/virnetworkportdef.c

@@ -75,7 +75,7 @@ virNetworkPortDefFree(virNetworkPortDefPtr def)
 static virNetworkPortDefPtr
 virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
 {
-    virNetworkPortDefPtr def;
+    VIR_AUTOPTR(virNetworkPortDef) def = NULL;
     VIR_AUTOFREE(char *) uuid = NULL;
     xmlNodePtr virtPortNode;
     xmlNodePtr vlanNode;
@@ -96,19 +96,19 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
     if (!uuid) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("network port has no uuid"));
-        goto error;
+        return NULL;
     }
     if (virUUIDParse(uuid, def->uuid) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Unable to parse UUID '%s'"), uuid);
-        goto error;
+        return NULL;
     }
 
     def->ownername = virXPathString("string(./owner/name)", ctxt);
     if (!def->ownername) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("network port has no owner name"));
-        goto error;
+        return NULL;
     }
 
     VIR_FREE(uuid);
@@ -116,13 +116,13 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
     if (!uuid) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("network port has no owner UUID"));
-        goto error;
+        return NULL;
     }
 
     if (virUUIDParse(uuid, def->owneruuid) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Unable to parse UUID '%s'"), uuid);
-        goto error;
+        return NULL;
     }
 
     def->group = virXPathString("string(./group)", ctxt);
@@ -130,19 +130,19 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
     virtPortNode = virXPathNode("./virtualport", ctxt);
     if (virtPortNode &&
         (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode, 0)))) {
-        goto error;
+        return NULL;
     }
 
     mac = virXPathString("string(./mac/@address)", ctxt);
     if (!mac) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("network port has no mac"));
-        goto error;
+        return NULL;
     }
     if (virMacAddrParse(mac, &def->mac) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Unable to parse MAC '%s'"), mac);
-        goto error;
+        return NULL;
     }
 
     bandwidthNode = virXPathNode("./bandwidth", ctxt);
@@ -155,11 +155,11 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
     if (bandwidthNode &&
         virNetDevBandwidthParse(&def->bandwidth, &def->class_id,
                                 bandwidthNode, true) < 0)
-        goto error;
+        return NULL;
 
     vlanNode = virXPathNode("./vlan", ctxt);
     if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0)
-        goto error;
+        return NULL;
 
 
     trustGuestRxFilters
@@ -170,7 +170,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
             virReportError(VIR_ERR_XML_ERROR,
                            _("Invalid guest rx filters trust setting '%s' "),
                            trustGuestRxFilters);
-            goto error;
+            return NULL;
         }
     }
 
@@ -191,7 +191,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
         if (!(def->plug.bridge.brname = virXPathString("string(./plug/@bridge)", ctxt))) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Missing network port bridge name"));
-            goto error;
+            return NULL;
         }
         macmgr = virXPathString("string(./plug/@macTableManager)", ctxt);
         if (macmgr &&
@@ -200,7 +200,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
             virReportError(VIR_ERR_XML_ERROR,
                            _("Invalid macTableManager setting '%s' "
                              "in network port"), macmgr);
-            goto error;
+            return NULL;
         }
         break;
 
@@ -208,7 +208,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
         if (!(def->plug.direct.linkdev = virXPathString("string(./plug/@dev)", ctxt))) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Missing network port link device name"));
-            goto error;
+            return NULL;
         }
         mode = virXPathString("string(./plug/@mode)", ctxt);
         if (mode &&
@@ -216,7 +216,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
              virNetDevMacVLanModeTypeFromString(mode)) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("Invalid mode setting '%s' in network port"), mode);
-            goto error;
+            return NULL;
         }
         break;
 
@@ -227,7 +227,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
              virTristateBoolTypeFromString(managed)) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("Invalid managed setting '%s' in network port"), mode);
-            goto error;
+            return NULL;
         }
         driver = virXPathString("string(./plug/driver/@name)", ctxt);
         if (driver &&
@@ -235,31 +235,25 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
              virNetworkForwardDriverNameTypeFromString(driver)) <= 0) {
               virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Missing network port driver name"));
-            goto error;
+            return NULL;
         }
         if (!(addressNode = virXPathNode("./plug/address", ctxt))) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Missing network port PCI address"));
-            goto error;
+            return NULL;
         }
 
         if (virPCIDeviceAddressParseXML(addressNode, &def->plug.hostdevpci.addr) < 0)
-            goto error;
+            return NULL;
         break;
 
     case VIR_NETWORK_PORT_PLUG_TYPE_LAST:
     default:
         virReportEnumRangeError(virNetworkPortPlugType, def->plugtype);
-        goto error;
+        return NULL;
     }
 
- cleanup:
-    return def;
-
- error:
-    virNetworkPortDefFree(def);
-    def = NULL;
-    goto cleanup;
+    VIR_RETURN_PTR(def);
 }
 
 

src/conf/virnetworkportdef.h

@@ -82,6 +82,7 @@ struct _virNetworkPortDef {
 
 void
 virNetworkPortDefFree(virNetworkPortDefPtr port);
+VIR_DEFINE_AUTOPTR_FUNC(virNetworkPortDef, virNetworkPortDefFree);
 
 virNetworkPortDefPtr
 virNetworkPortDefParseNode(xmlDocPtr xml,

src/network/bridge_driver.c

@@ -5560,7 +5560,7 @@ networkPortCreateXML(virNetworkPtr net,
     virNetworkDriverStatePtr driver = networkGetDriver();
     virNetworkObjPtr obj;
     virNetworkDefPtr def;
-    virNetworkPortDefPtr portdef = NULL;
+    VIR_AUTOPTR(virNetworkPortDef) portdef = NULL;
     virNetworkPortPtr ret = NULL;
     int rc;
 
@@ -5610,13 +5610,13 @@ networkPortCreateXML(virNetworkPtr net,
 
         virErrorPreserveLast(&save_err);
         ignore_value(networkReleasePort(obj, portdef));
-        virNetworkPortDefFree(portdef);
         virErrorRestore(&save_err);
 
         goto cleanup;
     }
 
     ret = virGetNetworkPort(net, portdef->uuid);
+    portdef = NULL;
  cleanup:
     virNetworkObjEndAPI(&obj);
     return ret;

tests/virnetworkportxml2xmltest.c

@@ -38,7 +38,7 @@ testCompareXMLToXMLFiles(const char *expected)
 {
     char *actual = NULL;
     int ret = -1;
-    virNetworkPortDefPtr dev = NULL;
+    VIR_AUTOPTR(virNetworkPortDef) dev = NULL;
 
     if (!(dev = virNetworkPortDefParseFile(expected)))
         goto cleanup;
@@ -52,7 +52,6 @@ testCompareXMLToXMLFiles(const char *expected)
     ret = 0;
  cleanup:
     VIR_FREE(actual);
-    virNetworkPortDefFree(dev);
     return ret;
 }