Re: [libvirt PATCH 01/10] virNodeDeviceDefParseXML: Use g_auto*
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/node_device_conf.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4477a8d9d2..5ac046f768 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2059,21 +2059,19 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
int create,
const char *virt_type)
{
- virNodeDeviceDef *def;
+ g_autoptr(virNodeDeviceDef) def = g_new0(virNodeDeviceDef, 1);
virNodeDevCapsDef **next_cap;
- xmlNodePtr *nodes = NULL;
+ g_autofree xmlNodePtr *nodes = NULL;
int n, m;
size_t i;
- def = g_new0(virNodeDeviceDef, 1);
-
/* Extract device name */
if (create == EXISTING_DEVICE) {
def->name = virXPathString("string(./name[1])", ctxt);
if (!def->name) {
virReportError(VIR_ERR_NO_NAME, NULL);
- goto error;
+ return NULL;
}
} else {
def->name = g_strdup("new device");
@@ -2083,7 +2081,7 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
/* Parse devnodes */
if ((n = virXPathNodeSet("./devnode", ctxt, &nodes)) < 0)
- goto error;
+ return NULL;
def->devlinks = g_new0(char *, n + 1);
@@ -2093,16 +2091,16 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
if (virXMLPropEnum(node, "type", virNodeDevDevnodeTypeFromString,
VIR_XML_PROP_REQUIRED, &val) < 0)
- goto error;
+ return NULL;
switch (val) {
case VIR_NODE_DEV_DEVNODE_DEV:
if (!(def->devnode = virXMLNodeContentString(node)))
- goto error;
+ return NULL;
break;
case VIR_NODE_DEV_DEVNODE_LINK:
if (!(def->devlinks[m++] = virXMLNodeContentString(node)))
- goto error;
+ return NULL;
break;
case VIR_NODE_DEV_DEVNODE_LAST:
break;
@@ -2118,7 +2116,7 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
_("when providing parent wwnn='%s', the "
"wwpn must also be provided"),
def->parent_wwnn);
- goto error;
+ return NULL;
}
if (!def->parent_wwnn && def->parent_wwpn) {
@@ -2126,7 +2124,7 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
_("when providing parent wwpn='%s', the "
"wwnn must also be provided"),
def->parent_wwpn);
- goto error;
+ return NULL;
}
def->parent_fabric_wwn =
virXPathString("string(./parent[1]/@fabric_wwn)",
ctxt);
@@ -2134,13 +2132,13 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
/* Parse device capabilities */
VIR_FREE(nodes);
The presence of a VIR_FREE() of a pointer that was declared g_autofree
points out that it's being re-used, which we had decided is a bad idea
(unless more discussion has happened on the topic that I missed). You
should instead define two separate g_autofree pointers, one for each use.
if ((n = virXPathNodeSet("./capability", ctxt,
&nodes)) < 0)
- goto error;
+ return NULL;
if (n == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("no device capabilities for '%s'"),
def->name);
- goto error;
+ return NULL;
}
next_cap = &def->caps;
@@ -2150,18 +2148,12 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
create,
virt_type);
if (!*next_cap)
- goto error;
+ return NULL;
next_cap = &(*next_cap)->next;
}
- VIR_FREE(nodes);
- return def;
-
- error:
- virNodeDeviceDefFree(def);
- VIR_FREE(nodes);
- return NULL;
+ return g_steal_pointer(&def);
}