Re: [PATCH 2/4] conf: Use VIR_AUTOPTR(virBitmap) in domain_conf

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Wed, Feb 20, 2019 at 09:46:57AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.

My clang 7 and gcc 8.2 both happily compile code where the cleanup:
label only contains a return statement, the goto paths can be cleaned
up later.

> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 53 ++++++++++++++++--------------------------
> 1 file changed, 20 insertions(+), 33 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ceeb247ef4..ddcb76f05d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1803,8 +1803,8 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>                                  virBitmapPtr autoCpuset)
> {
>     int maxvcpus = virDomainDefGetVcpusMax(def);
> -    virBitmapPtr allcpumap = NULL;
>     size_t i;
> +    VIR_AUTOPTR(virBitmap) allcpumap = NULL;
>
>     if (hostcpus < 0)
>         return -1;
> @@ -1831,7 +1831,6 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>         virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, i), maplen);
>     }
>
> -    virBitmapFree(allcpumap);
>     return i;
> }
>
> @@ -2984,11 +2983,10 @@ static int
> virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>                                 unsigned int iothreads)
> {
> -    int retval = -1;
>     size_t i;
>     ssize_t nxt = -1;
>     virDomainIOThreadIDDefPtr iothrid = NULL;
> -    virBitmapPtr thrmap = NULL;
> +    VIR_AUTOPTR(virBitmap) thrmap = NULL;
>
>     /* Same value (either 0 or some number), then we have none to fill in or
>      * the iothreadid array was filled from the XML
> @@ -2998,7 +2996,7 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>
>     /* iothread's are numbered starting at 1, account for that */
>     if (!(thrmap = virBitmapNew(iothreads + 1)))
> -        goto error;
> +        return -1;
>     virBitmapSetAll(thrmap);
>
>     /* Clear 0 since we don't use it, then mark those which are
> @@ -3010,27 +3008,23 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>
>     /* resize array */
>     if (VIR_REALLOC_N(def->iothreadids, iothreads) < 0)
> -        goto error;
> +        return -1;
>
>     /* Populate iothreadids[] using the set bit number from thrmap */
>     while (def->niothreadids < iothreads) {
>         if ((nxt = virBitmapNextSetBit(thrmap, nxt)) < 0) {
>             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                            _("failed to populate iothreadids"));
> -            goto error;
> +            return -1;
>         }
>         if (VIR_ALLOC(iothrid) < 0)
> -            goto error;
> +            return -1;
>         iothrid->iothread_id = nxt;
>         iothrid->autofill = true;
>         def->iothreadids[def->niothreadids++] = iothrid;
>     }
>
> -    retval = 0;
> -
> - error:
> -    virBitmapFree(thrmap);
> -    return retval;
> +    return 0;
> }
>
>
> @@ -18327,9 +18321,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
> {
>     int ret = -1;
>     virDomainIOThreadIDDefPtr iothrid;
> -    virBitmapPtr cpumask = NULL;
>     unsigned int iothreadid;
>     char *tmp = NULL;
> +    VIR_AUTOPTR(virBitmap) cpumask = NULL;
>
>     if (!(tmp = virXMLPropString(node, "iothread"))) {
>         virReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -18385,7 +18379,6 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
>
>  cleanup:
>     VIR_FREE(tmp);
> -    virBitmapFree(cpumask);
>     return ret;
> }
>
> @@ -18397,8 +18390,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
> static virBitmapPtr
> virDomainEmulatorPinDefParseXML(xmlNodePtr node)
> {
> -    virBitmapPtr def = NULL;
> +    virBitmapPtr ret = NULL;
>     char *tmp = NULL;
> +    VIR_AUTOPTR(virBitmap) def = NULL;
>
>     if (!(tmp = virXMLPropString(node, "cpuset"))) {
>         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -18412,14 +18406,14 @@ virDomainEmulatorPinDefParseXML(xmlNodePtr node)
>     if (virBitmapIsAllClear(def)) {
>         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                        _("Invalid value of 'cpuset': %s"), tmp);
> -        virBitmapFree(def);
> -        def = NULL;
>         goto cleanup;
>     }
>
> +    VIR_STEAL_PTR(ret, def);
> +

This 'ret' addition can also be separate.

Jano

Attachment: signature.asc
Description: PGP signature