[libvirt] [PATCH v2 12/32] storage: Use VIR_AUTOFREE for storage backends

Erik Skultety eskultet at redhat.com
Mon Feb 11 10:39:44 UTC 2019


On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities. This also allows
> for the cleanup of some goto paths.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---

> @@ -459,15 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
>                                              void *data)
>  {
>      virStoragePoolSourceListPtr sourceList = data;
> -    char *pvname = NULL;
> -    char *vgname = NULL;
>      size_t i;
>      virStoragePoolSourceDevicePtr dev;
>      virStoragePoolSource *thisSource;
> +    VIR_AUTOFREE(char *) pvname = NULL;
> +    VIR_AUTOFREE(char *) vgname = NULL;
>
>      if (VIR_STRDUP(pvname, groups[0]) < 0 ||
>          VIR_STRDUP(vgname, groups[1]) < 0)
> -        goto error;
> +        return -1;
>
>      thisSource = NULL;
>      for (i = 0; i < sourceList->nsources; i++) {
> @@ -479,30 +474,22 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
>
>      if (thisSource == NULL) {
>          if (!(thisSource = virStoragePoolSourceListNewSource(sourceList)))
> -            goto error;
> +            return -1;
>
> -        thisSource->name = vgname;
> +        VIR_STEAL_PTR(thisSource->name, vgname);
>      }
> -    else
> -        VIR_FREE(vgname);
>
>      if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0)
> -        goto error;
> +        return -1;
>
>      dev = &thisSource->devices[thisSource->ndevice];
>      thisSource->ndevice++;
>      thisSource->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
>
>      memset(dev, 0, sizeof(*dev));
> -    dev->path = pvname;
> +    VIR_STEAL_PTR(dev->path, pvname);

I still don't see why ^this needs to stay and can't be separated into a
preceding patch.

>
>      return 0;
> -
> - error:
> -    VIR_FREE(pvname);
> -    VIR_FREE(vgname);
> -
> -    return -1;
>  }

...

>
> diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c
> index f8bbde8cfe..7c2189d297 100644
> --- a/src/storage/storage_file_gluster.c
> +++ b/src/storage/storage_file_gluster.c
> @@ -258,10 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>                                               void *data)
>  {
>      virStorageFileBackendGlusterPrivPtr priv = data;
> -    char *buf = NULL;
>      size_t bufsiz = 0;
>      ssize_t ret;
>      struct stat st;
> +    VIR_AUTOFREE(char *) buf = NULL;
>
>      *linkpath = NULL;
>
> @@ -277,13 +277,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>
>   realloc:
>      if (VIR_EXPAND_N(buf, bufsiz, 256) < 0)
> -        goto error;
> +        return -1;
>
>      if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) {
>          virReportSystemError(errno,
>                               _("failed to read link of gluster file '%s'"),
>                               path);
> -        goto error;
> +        return -1;
>      }
>
>      if (ret == bufsiz)
> @@ -291,13 +291,9 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>
>      buf[ret] = '\0';
>
> -    *linkpath = buf;
> +    VIR_STEAL_PTR(*linkpath, buf);

^This VIR_STEAL_PTR can also be separated, it doesn't depend on the
VIR_AUTOFREE stuff, it's a simple 1 change hunk.

Erik




More information about the libvir-list mailing list