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 - 2026 Red Hat, Inc.