[libvirt PATCH 01/10] virNodeDeviceDefParseXML: Use g_auto*

[Laine Stump]

On 5/11/21 11:01 AM, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh at 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);
>   }
>   
>   
>