[libvirt] [PATCH 0/9] conf: Clean up XPath context handling

Refactor allocation and deallocation of the XPath context to have slightly less messy code. Peter Krempa (9): util: xml: Add wrapper for xmlXPathNewContext conf: network: Use VIR_AUTOPTR in virNetworkDefUpdateSection conf: Refactor resource cleanup in virDomainDeviceDefParse conf: Use automatic pointers for xmlXPathContext conf: Use VIR_AUTOPTR for xmlDoc and xmlXPath objects conf: domain: Register VIR_AUTOPTR function for virDomainDef conf: domain: Refactor cleanup in virDomainDefParseNode conf: secret: Register VIR_AUTOPTR function for virSecretDef conf: secret: Refactor cleanup in secretXMLParseNode src/conf/checkpoint_conf.c | 17 ++-- src/conf/domain_conf.c | 131 ++++++++++++------------------- src/conf/domain_conf.h | 1 + src/conf/interface_conf.c | 16 +--- src/conf/network_conf.c | 52 +++++------- src/conf/node_device_conf.c | 16 +--- src/conf/nwfilter_conf.c | 18 ++--- src/conf/secret_conf.c | 40 ++++------ src/conf/secret_conf.h | 2 + src/conf/snapshot_conf.c | 21 ++--- src/conf/storage_conf.c | 54 ++++--------- src/conf/virnetworkportdef.c | 5 +- src/conf/virnwfilterbindingdef.c | 5 +- src/conf/virnwfilterbindingobj.c | 5 +- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 4 +- src/util/virxml.c | 20 ++++- src/util/virxml.h | 3 + src/vbox/vbox_snapshot_conf.c | 17 ++-- src/vz/vz_sdk.c | 6 +- 20 files changed, 166 insertions(+), 268 deletions(-) -- 2.21.0

The wrapper reports libvirt errors for the libxml2 function so that the same does not have to be repeated over and over. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 5 +---- src/conf/domain_conf.c | 9 ++------- src/conf/interface_conf.c | 5 +---- src/conf/network_conf.c | 5 +---- src/conf/node_device_conf.c | 5 +---- src/conf/nwfilter_conf.c | 5 +---- src/conf/secret_conf.c | 6 ++---- src/conf/snapshot_conf.c | 5 +---- src/conf/storage_conf.c | 10 ++-------- src/conf/virnetworkportdef.c | 5 +---- src/conf/virnwfilterbindingdef.c | 5 +---- src/conf/virnwfilterbindingobj.c | 5 +---- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 4 +--- src/util/virxml.c | 20 ++++++++++++++++---- src/util/virxml.h | 3 +++ src/vbox/vbox_snapshot_conf.c | 17 ++++++++--------- src/vz/vz_sdk.c | 6 ++---- 18 files changed, 46 insertions(+), 75 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index a3513aaa65..113d85cc14 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -227,11 +227,8 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml, if (virXMLValidateAgainstSchema(schema, xml) < 0) return NULL; - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virDomainCheckpointDefParse(ctxt, caps, xmlopt, parseOpaque, flags); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 848c831330..e5e3f31f76 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21575,11 +21575,8 @@ virDomainDefParseNode(xmlDocPtr xml, virDomainDefPtr def = NULL; virDomainDefPtr ret = NULL; - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; @@ -21621,10 +21618,8 @@ virDomainObjParseNode(xmlDocPtr xml, goto cleanup; } - if (!(ctxt = xmlXPathNewContext(xml))) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; obj = virDomainObjParseXML(xml, ctxt, caps, xmlopt, flags); diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index b134ff4adc..64729aea43 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -830,11 +830,8 @@ virInterfaceDefParseNode(xmlDocPtr xml, return NULL; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST); diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index aa0d48af8f..585c87a9f4 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2125,11 +2125,8 @@ virNetworkDefParseNode(xmlDocPtr xml, return NULL; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virNetworkDefParseXML(ctxt, xmlopt); diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 618ce8e00e..8f8830aef1 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2063,11 +2063,8 @@ virNodeDeviceDefParseNode(xmlDocPtr xml, return NULL; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virNodeDeviceDefParseXML(ctxt, create, virt_type); diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index cecee51d87..21885eb7ae 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2756,11 +2756,8 @@ virNWFilterDefParseNode(xmlDocPtr xml, goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virNWFilterDefParseXML(ctxt); diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index b291339e77..a7691da7b5 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -144,11 +144,9 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } + ctxt->node = root; if (VIR_ALLOC(def) < 0) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index cce9a7999c..8cf80aed83 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -450,11 +450,8 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml, goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, parseOpaque, current, flags); diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 024f047fab..0560994281 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1024,11 +1024,8 @@ virStoragePoolDefParseNode(xmlDocPtr xml, goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virStoragePoolDefParseXML(ctxt); @@ -1468,11 +1465,8 @@ virStorageVolDefParseNode(virStoragePoolDefPtr pool, goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virStorageVolDefParseXML(pool, ctxt, flags); diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 29ecf5b178..2e20bff66e 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -277,11 +277,8 @@ virNetworkPortDefParseNode(xmlDocPtr xml, goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virNetworkPortDefParseXML(ctxt); diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index facca61833..6f13bc581a 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -182,11 +182,8 @@ virNWFilterBindingDefParseNode(xmlDocPtr xml, goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } ctxt->node = root; def = virNWFilterBindingDefParseXML(ctxt); diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c index 68afb9c434..06dd66a5d1 100644 --- a/src/conf/virnwfilterbindingobj.c +++ b/src/conf/virnwfilterbindingobj.c @@ -248,11 +248,8 @@ virNWFilterBindingObjParseNode(xmlDocPtr doc, goto cleanup; } - ctxt = xmlXPathNewContext(doc); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(doc))) goto cleanup; - } ctxt->node = root; obj = virNWFilterBindingObjParseXML(doc, ctxt); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4865edaf59..cd880d2c43 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3366,6 +3366,7 @@ virXMLValidateAgainstSchema; virXMLValidatorFree; virXMLValidatorInit; virXMLValidatorValidate; +virXMLXPathContextNew; virXPathBoolean; virXPathContextNodeRestore; virXPathInt; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9b19930964..01d3b008f0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3612,10 +3612,8 @@ virQEMUCapsLoadCache(virArch hostArch, if (!(doc = virXMLParseFile(filename))) goto cleanup; - if (!(ctxt = xmlXPathNewContext(doc))) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(doc))) goto cleanup; - } ctxt->node = xmlDocGetRootElement(doc); diff --git a/src/util/virxml.c b/src/util/virxml.c index f9c117dd58..245ca0a752 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -46,6 +46,20 @@ struct virParserData { }; +xmlXPathContextPtr +virXMLXPathContextNew(xmlDocPtr xml) +{ + xmlXPathContextPtr ctxt; + + if (!(ctxt = xmlXPathNewContext(xml))) { + virReportOOMError(); + return NULL; + } + + return ctxt; +} + + /** * virXPathString: * @xpath: the XPath string to evaluate @@ -824,11 +838,9 @@ virXMLParseHelper(int domcode, } if (ctxt) { - *ctxt = xmlXPathNewContext(xml); - if (!*ctxt) { - virReportOOMError(); + if (!(*ctxt = virXMLXPathContextNew(xml))) goto error; - } + (*ctxt)->node = xmlDocGetRootElement(xml); } diff --git a/src/util/virxml.h b/src/util/virxml.h index 30cc895436..39e5a988a2 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -30,6 +30,9 @@ #include "virbuffer.h" #include "virautoclean.h" +xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) + ATTRIBUTE_RETURN_CHECK; + int virXPathBoolean(const char *xpath, xmlXPathContextPtr ctxt); char * virXPathString(const char *xpath, diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index a36da30a4f..a6daf0ffcf 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -615,10 +615,9 @@ virVBoxSnapshotConfLoadVboxFile(const char *filePath, _("Unable to parse the xml")); goto cleanup; } - if (!(xPathContext = xmlXPathNewContext(xml))) { - virReportOOMError(); + if (!(xPathContext = virXMLXPathContextNew(xml))) goto cleanup; - } + if (xmlXPathRegisterNs(xPathContext, BAD_CAST "vbox", BAD_CAST "http://www.innotek.de/VirtualBox-settings") < 0) { @@ -1299,10 +1298,10 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, _("Unable to parse the xml")); goto cleanup; } - if (!(xPathContext = xmlXPathNewContext(xml))) { - virReportOOMError(); + + if (!(xPathContext = virXMLXPathContextNew(xml))) goto cleanup; - } + xPathContext->node = xmlDocGetRootElement(xml); if ((nodeSize = virXPathNodeSet("/domainsnapshot/disks/disk", xPathContext, &nodes)) < 0) @@ -1360,10 +1359,10 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, _("Unable to parse the xml")); goto cleanup; } - if (!(xPathContext = xmlXPathNewContext(xml))) { - virReportOOMError(); + + if (!(xPathContext = virXMLXPathContextNew(xml))) goto cleanup; - } + xPathContext->node = xmlDocGetRootElement(xml); if ((nodeSize = virXPathNodeSet("/domainsnapshot/domain/devices/disk", xPathContext, diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 478443298f..c6e3398620 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4674,11 +4674,9 @@ prlsdkParseSnapshotTree(const char *treexml) goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(); + if (!(ctxt = virXMLXPathContextNew(xml))) goto cleanup; - } + ctxt->node = root; if ((n = virXPathNodeSet("//SavedStateItem", ctxt, &nodes)) < 0) { -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:04 +0200, Peter Krempa wrote:
The wrapper reports libvirt errors for the libxml2 function so that the same does not have to be repeated over and over.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 5 +---- src/conf/domain_conf.c | 9 ++------- src/conf/interface_conf.c | 5 +---- src/conf/network_conf.c | 5 +---- src/conf/node_device_conf.c | 5 +---- src/conf/nwfilter_conf.c | 5 +---- src/conf/secret_conf.c | 6 ++---- src/conf/snapshot_conf.c | 5 +---- src/conf/storage_conf.c | 10 ++-------- src/conf/virnetworkportdef.c | 5 +---- src/conf/virnwfilterbindingdef.c | 5 +---- src/conf/virnwfilterbindingobj.c | 5 +---- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 4 +--- src/util/virxml.c | 20 ++++++++++++++++---- src/util/virxml.h | 3 +++ src/vbox/vbox_snapshot_conf.c | 17 ++++++++--------- src/vz/vz_sdk.c | 6 ++---- 18 files changed, 46 insertions(+), 75 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Add automatic cleanup for variables of xmlDoc and xmlXPathContext type to remove the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/network_conf.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 585c87a9f4..b65fb1f67a 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3640,57 +3640,56 @@ virNetworkDefUpdateSection(virNetworkDefPtr def, const char *xml, unsigned int flags) /* virNetworkUpdateFlags */ { - int ret = -1; - xmlDocPtr doc; - xmlXPathContextPtr ctxt = NULL; + VIR_AUTOPTR(xmlDoc) doc = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!(doc = virXMLParseStringCtxt(xml, _("network_update_xml"), &ctxt))) - goto cleanup; + return -1; switch (section) { case VIR_NETWORK_SECTION_BRIDGE: - ret = virNetworkDefUpdateBridge(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateBridge(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_DOMAIN: - ret = virNetworkDefUpdateDomain(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateDomain(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_IP: - ret = virNetworkDefUpdateIP(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateIP(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_IP_DHCP_HOST: - ret = virNetworkDefUpdateIPDHCPHost(def, command, + return virNetworkDefUpdateIPDHCPHost(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_IP_DHCP_RANGE: - ret = virNetworkDefUpdateIPDHCPRange(def, command, + return virNetworkDefUpdateIPDHCPRange(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_FORWARD: - ret = virNetworkDefUpdateForward(def, command, + return virNetworkDefUpdateForward(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_FORWARD_INTERFACE: - ret = virNetworkDefUpdateForwardInterface(def, command, + return virNetworkDefUpdateForwardInterface(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_FORWARD_PF: - ret = virNetworkDefUpdateForwardPF(def, command, + return virNetworkDefUpdateForwardPF(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_PORTGROUP: - ret = virNetworkDefUpdatePortGroup(def, command, + return virNetworkDefUpdatePortGroup(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_DNS_HOST: - ret = virNetworkDefUpdateDNSHost(def, command, + return virNetworkDefUpdateDNSHost(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_DNS_TXT: - ret = virNetworkDefUpdateDNSTxt(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateDNSTxt(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_DNS_SRV: - ret = virNetworkDefUpdateDNSSrv(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateDNSSrv(def, command, parentIndex, ctxt, flags); break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", @@ -3698,8 +3697,5 @@ virNetworkDefUpdateSection(virNetworkDefPtr def, break; } - cleanup: - xmlFreeDoc(doc); - xmlXPathFreeContext(ctxt); - return ret; + return -1; } -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:05 +0200, Peter Krempa wrote:
Add automatic cleanup for variables of xmlDoc and xmlXPathContext type to remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/network_conf.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 585c87a9f4..b65fb1f67a 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3640,57 +3640,56 @@ virNetworkDefUpdateSection(virNetworkDefPtr def, const char *xml, unsigned int flags) /* virNetworkUpdateFlags */ { - int ret = -1; - xmlDocPtr doc; - xmlXPathContextPtr ctxt = NULL; + VIR_AUTOPTR(xmlDoc) doc = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
if (!(doc = virXMLParseStringCtxt(xml, _("network_update_xml"), &ctxt))) - goto cleanup; + return -1;
switch (section) { case VIR_NETWORK_SECTION_BRIDGE: - ret = virNetworkDefUpdateBridge(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateBridge(def, command, parentIndex, ctxt, flags); break;
case VIR_NETWORK_SECTION_DOMAIN: - ret = virNetworkDefUpdateDomain(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateDomain(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_IP: - ret = virNetworkDefUpdateIP(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateIP(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_IP_DHCP_HOST: - ret = virNetworkDefUpdateIPDHCPHost(def, command, + return virNetworkDefUpdateIPDHCPHost(def, command, parentIndex, ctxt, flags);
I <Esc> and in several similar cases below...
break; case VIR_NETWORK_SECTION_IP_DHCP_RANGE: - ret = virNetworkDefUpdateIPDHCPRange(def, command, + return virNetworkDefUpdateIPDHCPRange(def, command, parentIndex, ctxt, flags);
.
break; case VIR_NETWORK_SECTION_FORWARD: - ret = virNetworkDefUpdateForward(def, command, + return virNetworkDefUpdateForward(def, command, parentIndex, ctxt, flags);
.
break; case VIR_NETWORK_SECTION_FORWARD_INTERFACE: - ret = virNetworkDefUpdateForwardInterface(def, command, + return virNetworkDefUpdateForwardInterface(def, command, parentIndex, ctxt, flags);
.
break; case VIR_NETWORK_SECTION_FORWARD_PF: - ret = virNetworkDefUpdateForwardPF(def, command, + return virNetworkDefUpdateForwardPF(def, command, parentIndex, ctxt, flags);
.
break; case VIR_NETWORK_SECTION_PORTGROUP: - ret = virNetworkDefUpdatePortGroup(def, command, + return virNetworkDefUpdatePortGroup(def, command, parentIndex, ctxt, flags);
.
break; case VIR_NETWORK_SECTION_DNS_HOST: - ret = virNetworkDefUpdateDNSHost(def, command, + return virNetworkDefUpdateDNSHost(def, command, parentIndex, ctxt, flags);
.
break; case VIR_NETWORK_SECTION_DNS_TXT: - ret = virNetworkDefUpdateDNSTxt(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateDNSTxt(def, command, parentIndex, ctxt, flags); break; case VIR_NETWORK_SECTION_DNS_SRV: - ret = virNetworkDefUpdateDNSSrv(def, command, parentIndex, ctxt, flags); + return virNetworkDefUpdateDNSSrv(def, command, parentIndex, ctxt, flags); break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", @@ -3698,8 +3697,5 @@ virNetworkDefUpdateSection(virNetworkDefPtr def, break; }
- cleanup: - xmlFreeDoc(doc); - xmlXPathFreeContext(ctxt); - return ret; + return -1; }
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Use VIR_AUTO* helpers to get rid of the convoluted cleanup path. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 73 +++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e5e3f31f76..2fe591c328 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16330,19 +16330,19 @@ virDomainDeviceDefParse(const char *xmlStr, void *parseOpaque, unsigned int flags) { - xmlDocPtr xml; + VIR_AUTOPTR(xmlDoc) xml = NULL; xmlNodePtr node; - xmlXPathContextPtr ctxt = NULL; - virDomainDeviceDefPtr dev = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; + VIR_AUTOFREE(virDomainDeviceDefPtr) dev = NULL; char *netprefix; if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) - goto error; + return NULL; node = ctxt->node; if (VIR_ALLOC(dev) < 0) - goto error; + return NULL; if ((dev->type = virDomainDeviceTypeFromString((const char *) node->name)) < 0) { /* Some crazy mapping of serial, parallel, console and channel to @@ -16356,7 +16356,7 @@ virDomainDeviceDefParse(const char *xmlStr, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown device type '%s'"), node->name); - goto error; + return NULL; } } @@ -16366,71 +16366,71 @@ virDomainDeviceDefParse(const char *xmlStr, def->seclabels, def->nseclabels, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_LEASE: if (!(dev->data.lease = virDomainLeaseDefParseXML(node))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_FS: if (!(dev->data.fs = virDomainFSDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_NET: netprefix = caps->host.netprefix; if (!(dev->data.net = virDomainNetDefParseXML(xmlopt, node, ctxt, netprefix, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_INPUT: if (!(dev->data.input = virDomainInputDefParseXML(xmlopt, def, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_SOUND: if (!(dev->data.sound = virDomainSoundDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_WATCHDOG: if (!(dev->data.watchdog = virDomainWatchdogDefParseXML(xmlopt, node, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_VIDEO: if (!(dev->data.video = virDomainVideoDefParseXML(xmlopt, node, ctxt, def, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_HOSTDEV: if (!(dev->data.hostdev = virDomainHostdevDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_CONTROLLER: if (!(dev->data.controller = virDomainControllerDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_GRAPHICS: if (!(dev->data.graphics = virDomainGraphicsDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_HUB: if (!(dev->data.hub = virDomainHubDefParseXML(xmlopt, node, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_REDIRDEV: if (!(dev->data.redirdev = virDomainRedirdevDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_RNG: if (!(dev->data.rng = virDomainRNGDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_CHR: if (!(dev->data.chr = virDomainChrDefParseXML(xmlopt, @@ -16439,50 +16439,50 @@ virDomainDeviceDefParse(const char *xmlStr, def->seclabels, def->nseclabels, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_SMARTCARD: if (!(dev->data.smartcard = virDomainSmartcardDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_MEMBALLOON: if (!(dev->data.memballoon = virDomainMemballoonDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_NVRAM: if (!(dev->data.nvram = virDomainNVRAMDefParseXML(xmlopt, node, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_SHMEM: if (!(dev->data.shmem = virDomainShmemDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_TPM: if (!(dev->data.tpm = virDomainTPMDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_PANIC: if (!(dev->data.panic = virDomainPanicDefParseXML(xmlopt, node, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_MEMORY: if (!(dev->data.memory = virDomainMemoryDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_IOMMU: if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_VSOCK: if (!(dev->data.vsock = virDomainVsockDefParseXML(xmlopt, node, ctxt, flags))) - goto error; + return NULL; break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: @@ -16492,20 +16492,13 @@ virDomainDeviceDefParse(const char *xmlStr, /* callback to fill driver specific device aspects */ if (virDomainDeviceDefPostParseOne(dev, def, caps, flags, xmlopt, parseOpaque) < 0) - goto error; + return NULL; /* validate the configuration */ if (virDomainDeviceDefValidate(dev, def, flags, xmlopt) < 0) - goto error; - - cleanup: - xmlFreeDoc(xml); - xmlXPathFreeContext(ctxt); - return dev; + return NULL; - error: - VIR_FREE(dev); - goto cleanup; + VIR_RETURN_PTR(dev); } -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:06 +0200, Peter Krempa wrote:
Use VIR_AUTO* helpers to get rid of the convoluted cleanup path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 73 +++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 40 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Clean up functions which grab and free the context to use VIR_AUTOPTR. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 12 ++++-------- src/conf/domain_conf.c | 13 ++++--------- src/conf/interface_conf.c | 11 +++-------- src/conf/network_conf.c | 11 +++-------- src/conf/node_device_conf.c | 11 +++-------- src/conf/nwfilter_conf.c | 13 ++++--------- src/conf/snapshot_conf.c | 16 ++++++---------- src/conf/storage_conf.c | 24 ++++++++---------------- 8 files changed, 35 insertions(+), 76 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 113d85cc14..5c998267aa 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -209,13 +209,12 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml, void *parseOpaque, unsigned int flags) { - xmlXPathContextPtr ctxt = NULL; - virDomainCheckpointDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; VIR_AUTOFREE(char *) schema = NULL; if (!virXMLNodeNameEqual(root, "domaincheckpoint")) { virReportError(VIR_ERR_XML_ERROR, "%s", _("domaincheckpoint")); - goto cleanup; + return NULL; } /* This is a new enough API to make schema validation unconditional */ @@ -228,13 +227,10 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml, return NULL; if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virDomainCheckpointDefParse(ctxt, caps, xmlopt, parseOpaque, flags); - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virDomainCheckpointDefParse(ctxt, caps, xmlopt, parseOpaque, flags); } virDomainCheckpointDefPtr diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2fe591c328..0ab69a9366 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21600,26 +21600,21 @@ virDomainObjParseNode(xmlDocPtr xml, virDomainXMLOptionPtr xmlopt, unsigned int flags) { - xmlXPathContextPtr ctxt = NULL; - virDomainObjPtr obj = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!virXMLNodeNameEqual(root, "domstatus")) { virReportError(VIR_ERR_XML_ERROR, _("unexpected root element <%s>, " "expecting <domstatus>"), root->name); - goto cleanup; + return NULL; } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - obj = virDomainObjParseXML(xml, ctxt, caps, xmlopt, flags); - - cleanup: - xmlXPathFreeContext(ctxt); - return obj; + return virDomainObjParseXML(xml, ctxt, caps, xmlopt, flags); } diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 64729aea43..2b65a235ea 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -819,8 +819,7 @@ virInterfaceDefPtr virInterfaceDefParseNode(xmlDocPtr xml, xmlNodePtr root) { - xmlXPathContextPtr ctxt = NULL; - virInterfaceDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!virXMLNodeNameEqual(root, "interface")) { virReportError(VIR_ERR_XML_ERROR, @@ -831,14 +830,10 @@ virInterfaceDefParseNode(xmlDocPtr xml, } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST); - - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST); } diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b65fb1f67a..9990d5e79d 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2114,8 +2114,7 @@ virNetworkDefParseNode(xmlDocPtr xml, xmlNodePtr root, virNetworkXMLOptionPtr xmlopt) { - xmlXPathContextPtr ctxt = NULL; - virNetworkDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!virXMLNodeNameEqual(root, "network")) { virReportError(VIR_ERR_XML_ERROR, @@ -2126,14 +2125,10 @@ virNetworkDefParseNode(xmlDocPtr xml, } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virNetworkDefParseXML(ctxt, xmlopt); - - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virNetworkDefParseXML(ctxt, xmlopt); } diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 8f8830aef1..84fcbd3e78 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2052,8 +2052,7 @@ virNodeDeviceDefParseNode(xmlDocPtr xml, int create, const char *virt_type) { - xmlXPathContextPtr ctxt = NULL; - virNodeDeviceDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!virXMLNodeNameEqual(root, "device")) { virReportError(VIR_ERR_XML_ERROR, @@ -2064,14 +2063,10 @@ virNodeDeviceDefParseNode(xmlDocPtr xml, } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virNodeDeviceDefParseXML(ctxt, create, virt_type); - - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virNodeDeviceDefParseXML(ctxt, create, virt_type); } diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 21885eb7ae..d21eebf19d 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2746,25 +2746,20 @@ virNWFilterDefPtr virNWFilterDefParseNode(xmlDocPtr xml, xmlNodePtr root) { - xmlXPathContextPtr ctxt = NULL; - virNWFilterDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (STRNEQ((const char *)root->name, "filter")) { virReportError(VIR_ERR_XML_ERROR, "%s", _("unknown root element for nw filter")); - goto cleanup; + return NULL; } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virNWFilterDefParseXML(ctxt); - - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virNWFilterDefParseXML(ctxt); } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 8cf80aed83..61c807a71f 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -430,12 +430,11 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml, bool *current, unsigned int flags) { - xmlXPathContextPtr ctxt = NULL; - virDomainSnapshotDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!virXMLNodeNameEqual(root, "domainsnapshot")) { virReportError(VIR_ERR_XML_ERROR, "%s", _("domainsnapshot")); - goto cleanup; + return NULL; } if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE) { @@ -445,19 +444,16 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml, abs_top_srcdir "/docs/schemas", PKGDATADIR "/schemas"); if (!schema) - goto cleanup; + return NULL; if (virXMLValidateAgainstSchema(schema, xml) < 0) - goto cleanup; + return NULL; } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, parseOpaque, current, flags); - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virDomainSnapshotDefParse(ctxt, caps, xmlopt, parseOpaque, current, flags); } virDomainSnapshotDefPtr diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0560994281..f6de3687ab 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1013,25 +1013,21 @@ virStoragePoolDefPtr virStoragePoolDefParseNode(xmlDocPtr xml, xmlNodePtr root) { - xmlXPathContextPtr ctxt = NULL; - virStoragePoolDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!virXMLNodeNameEqual(root, "pool")) { virReportError(VIR_ERR_XML_ERROR, _("unexpected root element <%s>, " "expecting <pool>"), root->name); - goto cleanup; + return NULL; } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virStoragePoolDefParseXML(ctxt); - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virStoragePoolDefParseXML(ctxt); } @@ -1454,25 +1450,21 @@ virStorageVolDefParseNode(virStoragePoolDefPtr pool, xmlNodePtr root, unsigned int flags) { - xmlXPathContextPtr ctxt = NULL; - virStorageVolDefPtr def = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; if (!virXMLNodeNameEqual(root, "volume")) { virReportError(VIR_ERR_XML_ERROR, _("unexpected root element <%s>, " "expecting <volume>"), root->name); - goto cleanup; + return NULL; } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; - def = virStorageVolDefParseXML(pool, ctxt, flags); - cleanup: - xmlXPathFreeContext(ctxt); - return def; + return virStorageVolDefParseXML(pool, ctxt, flags); } -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:07 +0200, Peter Krempa wrote:
Clean up functions which grab and free the context to use VIR_AUTOPTR.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 12 ++++-------- src/conf/domain_conf.c | 13 ++++--------- src/conf/interface_conf.c | 11 +++-------- src/conf/network_conf.c | 11 +++-------- src/conf/node_device_conf.c | 11 +++-------- src/conf/nwfilter_conf.c | 13 ++++--------- src/conf/snapshot_conf.c | 16 ++++++---------- src/conf/storage_conf.c | 24 ++++++++---------------- 8 files changed, 35 insertions(+), 76 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Refactor functions using these two object types together with VIR_AUTOPTR. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 +++++----------- src/conf/storage_conf.c | 20 +++++++------------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0ab69a9366..5fec2e5220 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16508,20 +16508,19 @@ virDomainDiskDefParse(const char *xmlStr, virDomainXMLOptionPtr xmlopt, unsigned int flags) { - xmlDocPtr xml; - xmlXPathContextPtr ctxt = NULL; - virDomainDiskDefPtr disk = NULL; + VIR_AUTOPTR(xmlDoc) xml = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; virSecurityLabelDefPtr *seclabels = NULL; size_t nseclabels = 0; if (!(xml = virXMLParseStringCtxt(xmlStr, _("(disk_definition)"), &ctxt))) - goto cleanup; + return NULL; if (!virXMLNodeNameEqual(ctxt->node, "disk")) { virReportError(VIR_ERR_XML_ERROR, _("expecting root element of 'disk', not '%s'"), ctxt->node->name); - goto cleanup; + return NULL; } if (def) { @@ -16529,13 +16528,8 @@ virDomainDiskDefParse(const char *xmlStr, nseclabels = def->nseclabels; } - disk = virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt, + return virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt, seclabels, nseclabels, flags); - - cleanup: - xmlFreeDoc(xml); - xmlXPathFreeContext(ctxt); - return disk; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index f6de3687ab..fcd1701d37 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -695,36 +695,30 @@ virStoragePoolSourcePtr virStoragePoolDefParseSourceString(const char *srcSpec, int pool_type) { - xmlDocPtr doc = NULL; + VIR_AUTOPTR(xmlDoc) doc = NULL; xmlNodePtr node = NULL; - xmlXPathContextPtr xpath_ctxt = NULL; - virStoragePoolSourcePtr ret = NULL; + VIR_AUTOPTR(xmlXPathContext) xpath_ctxt = NULL; VIR_AUTOPTR(virStoragePoolSource) def = NULL; if (!(doc = virXMLParseStringCtxt(srcSpec, _("(storage_source_specification)"), &xpath_ctxt))) - goto cleanup; + return NULL; if (VIR_ALLOC(def) < 0) - goto cleanup; + return NULL; if (!(node = virXPathNode("/source", xpath_ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("root element was not source")); - goto cleanup; + return NULL; } if (virStoragePoolDefParseSource(xpath_ctxt, def, pool_type, node) < 0) - goto cleanup; - - VIR_STEAL_PTR(ret, def); - cleanup: - xmlFreeDoc(doc); - xmlXPathFreeContext(xpath_ctxt); + return NULL; - return ret; + VIR_RETURN_PTR(def); } -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:08 +0200, Peter Krempa wrote:
Refactor functions using these two object types together with VIR_AUTOPTR.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 +++++----------- src/conf/storage_conf.c | 20 +++++++------------- 2 files changed, 12 insertions(+), 24 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b688ee2b83..57ce5b95dc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2892,6 +2892,7 @@ bool virDomainDefHasDeviceAddress(virDomainDefPtr def, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; void virDomainDefFree(virDomainDefPtr vm); +VIR_DEFINE_AUTOPTR_FUNC(virDomainDef, virDomainDefFree); virDomainChrSourceDefPtr virDomainChrSourceDefNew(virDomainXMLOptionPtr xmlopt); -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:09 +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.h | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Use VIR_AUTOPTR for temporary locals and get rid of the cleanup label. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5fec2e5220..35573c0aaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21558,32 +21558,26 @@ virDomainDefParseNode(xmlDocPtr xml, void *parseOpaque, unsigned int flags) { - xmlXPathContextPtr ctxt = NULL; - virDomainDefPtr def = NULL; - virDomainDefPtr ret = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; + VIR_AUTOPTR(virDomainDef) def = NULL; if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; if (!(def = virDomainDefParseXML(xml, ctxt, caps, xmlopt, flags))) - goto cleanup; + return NULL; /* callback to fill driver specific domain aspects */ if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque) < 0) - goto cleanup; + return NULL; /* validate configuration */ if (virDomainDefValidate(def, caps, flags, xmlopt) < 0) - goto cleanup; - - VIR_STEAL_PTR(ret, def); + return NULL; - cleanup: - virDomainDefFree(def); - xmlXPathFreeContext(ctxt); - return ret; + VIR_RETURN_PTR(def); } -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:10 +0200, Peter Krempa wrote:
Use VIR_AUTOPTR for temporary locals and get rid of the cleanup label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index de1c28e187..7cfcfc8a60 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -35,6 +35,8 @@ struct _virSecretDef { }; void virSecretDefFree(virSecretDefPtr def); +VIR_DEFINE_AUTOPTR_FUNC(virSecretDef, virSecretDefFree); + virSecretDefPtr virSecretDefParseString(const char *xml); virSecretDefPtr virSecretDefParseFile(const char *filename); char *virSecretDefFormat(const virSecretDef *def); -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:11 +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index de1c28e187..7cfcfc8a60 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -35,6 +35,8 @@ struct _virSecretDef { };
void virSecretDefFree(virSecretDefPtr def); +VIR_DEFINE_AUTOPTR_FUNC(virSecretDef, virSecretDefFree); + virSecretDefPtr virSecretDefParseString(const char *xml); virSecretDefPtr virSecretDefParseFile(const char *filename); char *virSecretDefFormat(const virSecretDef *def);
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Use VIR_AUTO* for temporary locals and get rid of the 'cleanup' label. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index a7691da7b5..da61c6917e 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -131,33 +131,33 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, static virSecretDefPtr secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) { - xmlXPathContextPtr ctxt = NULL; - virSecretDefPtr def = NULL, ret = NULL; - char *prop = NULL; - char *uuidstr = NULL; + VIR_AUTOPTR(xmlXPathContext) ctxt = NULL; + VIR_AUTOPTR(virSecretDef) def = NULL; + VIR_AUTOFREE(char *) prop = NULL; + VIR_AUTOFREE(char *) uuidstr = NULL; if (!virXMLNodeNameEqual(root, "secret")) { virReportError(VIR_ERR_XML_ERROR, _("unexpected root element <%s>, " "expecting <secret>"), root->name); - goto cleanup; + return NULL; } if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; + return NULL; ctxt->node = root; if (VIR_ALLOC(def) < 0) - goto cleanup; + return NULL; prop = virXPathString("string(./@ephemeral)", ctxt); if (prop != NULL) { if (virStringParseYesNo(prop, &def->isephemeral) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'ephemeral'")); - goto cleanup; + return NULL; } VIR_FREE(prop); } @@ -167,7 +167,7 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) if (virStringParseYesNo(prop, &def->isprivate) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'private'")); - goto cleanup; + return NULL; } VIR_FREE(prop); } @@ -177,13 +177,13 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) if (virUUIDGenerate(def->uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); - goto cleanup; + return NULL; } } else { if (virUUIDParse(uuidstr, def->uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed uuid element")); - goto cleanup; + return NULL; } VIR_FREE(uuidstr); } @@ -191,15 +191,9 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) def->description = virXPathString("string(./description)", ctxt); if (virXPathNode("./usage", ctxt) != NULL && virSecretDefParseUsage(ctxt, def) < 0) - goto cleanup; - VIR_STEAL_PTR(ret, def); - - cleanup: - VIR_FREE(prop); - VIR_FREE(uuidstr); - virSecretDefFree(def); - xmlXPathFreeContext(ctxt); - return ret; + return NULL; + + VIR_RETURN_PTR(def); } static virSecretDefPtr -- 2.21.0

On Mon, Sep 16, 2019 at 14:17:12 +0200, Peter Krempa wrote:
Use VIR_AUTO* for temporary locals and get rid of the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Jiri Denemark
-
Peter Krempa