From b6a8d303027503be96ffb0e8ef0a080283f1bae1 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Sun, 22 Sep 2019 21:03:51 -0400 Subject: [PATCH] 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 Reviewed-by: Michal Privoznik --- src/conf/domain_conf.c | 107 ++++++++++++------------------ src/conf/virnetworkobj.c | 3 +- src/conf/virnetworkportdef.c | 50 ++++++-------- src/conf/virnetworkportdef.h | 1 + src/network/bridge_driver.c | 4 +- tests/virnetworkportxml2xmltest.c | 3 +- 6 files changed, 70 insertions(+), 98 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7d83a3d1c6..d2f421a528 100644 --- a/src/conf/domain_conf.c +++ b/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 diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index ca1d598cf9..69a962b466 100644 --- a/src/conf/virnetworkobj.c +++ b/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; } diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 2e20bff66e..3e43da064e 100644 --- a/src/conf/virnetworkportdef.c +++ b/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); } diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h index 3d42b9b6a2..796e269fe0 100644 --- a/src/conf/virnetworkportdef.h +++ b/src/conf/virnetworkportdef.h @@ -82,6 +82,7 @@ struct _virNetworkPortDef { void virNetworkPortDefFree(virNetworkPortDefPtr port); +VIR_DEFINE_AUTOPTR_FUNC(virNetworkPortDef, virNetworkPortDefFree); virNetworkPortDefPtr virNetworkPortDefParseNode(xmlDocPtr xml, diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c617bbb58f..ae2e4f09f8 100644 --- a/src/network/bridge_driver.c +++ b/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; diff --git a/tests/virnetworkportxml2xmltest.c b/tests/virnetworkportxml2xmltest.c index bb0ae8a8d5..9cbb327d92 100644 --- a/tests/virnetworkportxml2xmltest.c +++ b/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; } -- GitLab