src/conf/domain_conf.c | 58 ++++++++++++++----------------- src/conf/virnetworkobj.c | 3 +- src/conf/virnetworkportdef.c | 52 +++++++++++++-------------- tests/virnetworkportxml2xmltest.c | 3 +- 4 files changed, 53 insertions(+), 63 deletions(-)
Since the VIR_DEFINE_AUTOPTR_FUNC() was added for
virNetworkPortDefPtr, I decided to convert all uses of
virNetworkPortDefPtr that were appropriate to use VIR_AUTOPTR. This could be
squashed into patch 1/2, or left separate, or just completely dropped.
Signed-off-by: Laine Stump <laine@redhat.com>
---
src/conf/domain_conf.c | 58 ++++++++++++++-----------------
src/conf/virnetworkobj.c | 3 +-
src/conf/virnetworkportdef.c | 52 +++++++++++++--------------
tests/virnetworkportxml2xmltest.c | 3 +-
4 files changed, 53 insertions(+), 63 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d1e7ac84e8..d638c455d0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30565,7 +30565,8 @@ virNetworkPortDefPtr
virDomainNetDefToNetworkPort(virDomainDefPtr dom,
virDomainNetDefPtr iface)
{
- virNetworkPortDefPtr port;
+ VIR_AUTOPTR(virNetworkPortDef) port = NULL;
+ virNetworkPortDefPtr portret = NULL;
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -30580,34 +30581,31 @@ 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_STEAL_PTR(portret, port);
+ return portret;
}
int
@@ -30728,7 +30726,8 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
virDomainNetDefPtr iface)
{
virDomainActualNetDefPtr actual;
- virNetworkPortDefPtr port;
+ VIR_AUTOPTR(virNetworkPortDef) port = NULL;
+ virNetworkPortDefPtr portret = NULL;
if (!iface->data.network.actual) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -30754,15 +30753,15 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
} else 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);
@@ -30771,7 +30770,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;
@@ -30779,7 +30778,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;
@@ -30787,7 +30786,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;
@@ -30798,7 +30797,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;
@@ -30824,7 +30823,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom,
default:
virReportEnumRangeError(virDomainHostdevSubsysPCIBackendType,
actual->data.hostdev.def.source.subsys.u.pci.backend);
- goto error;
+ return NULL;
}
break;
@@ -30840,31 +30839,28 @@ 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_STEAL_PTR(portret, port);
+ return portret;
}
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..36e2c2c444 100644
--- a/src/conf/virnetworkportdef.c
+++ b/src/conf/virnetworkportdef.c
@@ -75,7 +75,8 @@ virNetworkPortDefFree(virNetworkPortDefPtr def)
static virNetworkPortDefPtr
virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
{
- virNetworkPortDefPtr def;
+ VIR_AUTOPTR(virNetworkPortDef) def = NULL;
+ virNetworkPortDefPtr defret = NULL;
VIR_AUTOFREE(char *) uuid = NULL;
xmlNodePtr virtPortNode;
xmlNodePtr vlanNode;
@@ -96,19 +97,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 +117,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 +131,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 +156,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 +171,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
virReportError(VIR_ERR_XML_ERROR,
_("Invalid guest rx filters trust setting '%s' "),
trustGuestRxFilters);
- goto error;
+ return NULL;
}
}
@@ -191,7 +192,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 +201,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
virReportError(VIR_ERR_XML_ERROR,
_("Invalid macTableManager setting '%s' "
"in network port"), macmgr);
- goto error;
+ return NULL;
}
break;
@@ -208,7 +209,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 +217,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 +228,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 +236,26 @@ 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_STEAL_PTR(defret, def);
+ return defret;
}
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;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 9/23/19 3:08 AM, Laine Stump wrote: > Since the VIR_DEFINE_AUTOPTR_FUNC() was added for > virNetworkPortDefPtr, I decided to convert all uses of > virNetworkPortDefPtr that were appropriate to use VIR_AUTOPTR. This could be > squashed into patch 1/2, or left separate, or just completely dropped. > > Signed-off-by: Laine Stump <laine@redhat.com> > --- > src/conf/domain_conf.c | 58 ++++++++++++++----------------- > src/conf/virnetworkobj.c | 3 +- > src/conf/virnetworkportdef.c | 52 +++++++++++++-------------- > tests/virnetworkportxml2xmltest.c | 3 +- > 4 files changed, 53 insertions(+), 63 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index d1e7ac84e8..d638c455d0 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -30565,7 +30565,8 @@ virNetworkPortDefPtr > virDomainNetDefToNetworkPort(virDomainDefPtr dom, > virDomainNetDefPtr iface) > { > - virNetworkPortDefPtr port; > + VIR_AUTOPTR(virNetworkPortDef) port = NULL; > + virNetworkPortDefPtr portret = NULL; Here and in the rest of the patch you don need to introduce XXXret variable, because ... > > - return port; > - > - error: > - virNetworkPortDefFree(port); > - return NULL; > + VIR_STEAL_PTR(portret, port); > + return portret; .. you can just use VIR_RETURN_PTR(port); > } Also, there is one more occurrence of virNetworkPortDefFree() in networkPortCreateXML() in src/network/bridge_driver.c. In fact, code inspection says that virNetworkPortDef might be leaked there (for instance if checks involving @portdef in the middle of the function fail), so please squash this in: diff --git i/src/network/bridge_driver.c w/src/network/bridge_driver.c index c54be96407..f9ef7eeb6f 100644 --- i/src/network/bridge_driver.c +++ w/src/network/bridge_driver.c @@ -5571,7 +5571,7 @@ networkPortCreateXML(virNetworkPtr net, virNetworkDriverStatePtr driver = networkGetDriver(); virNetworkObjPtr obj; virNetworkDefPtr def; - virNetworkPortDefPtr portdef = NULL; + VIR_AUTOPTR(virNetworkPortDef) portdef = NULL; virNetworkPortPtr ret = NULL; int rc; @@ -5621,13 +5621,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; Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 9/26/19 2:52 AM, Michal Privoznik wrote: > On 9/23/19 3:08 AM, Laine Stump wrote: >> Since the VIR_DEFINE_AUTOPTR_FUNC() was added for >> virNetworkPortDefPtr, I decided to convert all uses of >> virNetworkPortDefPtr that were appropriate to use VIR_AUTOPTR. This >> could be >> squashed into patch 1/2, or left separate, or just completely dropped. >> >> Signed-off-by: Laine Stump <laine@redhat.com> >> --- >> src/conf/domain_conf.c | 58 ++++++++++++++----------------- >> src/conf/virnetworkobj.c | 3 +- >> src/conf/virnetworkportdef.c | 52 +++++++++++++-------------- >> tests/virnetworkportxml2xmltest.c | 3 +- >> 4 files changed, 53 insertions(+), 63 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index d1e7ac84e8..d638c455d0 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -30565,7 +30565,8 @@ virNetworkPortDefPtr >> virDomainNetDefToNetworkPort(virDomainDefPtr dom, >> virDomainNetDefPtr iface) >> { >> - virNetworkPortDefPtr port; >> + VIR_AUTOPTR(virNetworkPortDef) port = NULL; >> + virNetworkPortDefPtr portret = NULL; > > Here and in the rest of the patch you don need to introduce XXXret > variable, because ... > > >> - return port; >> - >> - error: >> - virNetworkPortDefFree(port); >> - return NULL; >> + VIR_STEAL_PTR(portret, port); >> + return portret; > > .. you can just use VIR_RETURN_PTR(port); Ah, I never noticed that macro. Exactly what I wanted :-) > >> } > > > Also, there is one more occurrence of virNetworkPortDefFree() in > networkPortCreateXML() in src/network/bridge_driver.c. In fact, code > inspection says that virNetworkPortDef might be leaked there (for > instance if checks involving @portdef in the middle of the function > fail), so please squash this in: > > diff --git i/src/network/bridge_driver.c w/src/network/bridge_driver.c > index c54be96407..f9ef7eeb6f 100644 > --- i/src/network/bridge_driver.c > +++ w/src/network/bridge_driver.c > @@ -5571,7 +5571,7 @@ networkPortCreateXML(virNetworkPtr net, > virNetworkDriverStatePtr driver = networkGetDriver(); > virNetworkObjPtr obj; > virNetworkDefPtr def; > - virNetworkPortDefPtr portdef = NULL; > + VIR_AUTOPTR(virNetworkPortDef) portdef = NULL; > virNetworkPortPtr ret = NULL; > int rc; > > @@ -5621,13 +5621,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; I had looked at that one (I used cscope to look at all of them, and for some reason had decided it was inappropriate to convert. But your suggestion shows that I was just looking at it wrong. So yes, that looks like a good idea. I'll make the changes before pushing. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.