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

[Ján Tomko]

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>
>---
> src/storage/storage_backend.c              |  9 +--
> src/storage/storage_backend_disk.c         | 62 ++++++-----------
> src/storage/storage_backend_fs.c           | 17 ++---
> src/storage/storage_backend_gluster.c      | 30 +++-----
> src/storage/storage_backend_iscsi.c        | 73 +++++++-------------
> src/storage/storage_backend_iscsi_direct.c | 36 ++++------
> src/storage/storage_backend_logical.c      | 35 +++-------
> src/storage/storage_backend_mpath.c        | 18 ++---
> src/storage/storage_backend_rbd.c          | 35 +++-------
> src/storage/storage_backend_scsi.c         | 79 ++++++++--------------
> src/storage/storage_backend_sheepdog.c     | 27 +++-----
> src/storage/storage_backend_vstorage.c     | 25 +++----
> src/storage/storage_backend_zfs.c          | 15 ++--
> src/storage/storage_file_gluster.c         | 16 ++---
> 14 files changed, 158 insertions(+), 319 deletions(-)
>
>@@ -223,10 +216,10 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec,
>         }
>         VIR_FREE(list.sources);
>     }
>+    /* NB: Not virString -like, managed by VIR_APPEND_ELEMENT */

I don't see the point of this comment.

>     for (i = 0; i < ntargets; i++)
>         VIR_FREE(targets[i]);
>     VIR_FREE(targets);
>-    VIR_FREE(portal);
>     return ret;
> }
>
>diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
>index 423f945fbc..b78eb726b2 100644
>--- a/src/storage/storage_backend_mpath.c
>+++ b/src/storage/storage_backend_mpath.c
>@@ -153,33 +153,32 @@ static int
> virStorageBackendCreateVols(virStoragePoolObjPtr pool,
>                             struct dm_names *names)
> {
>-    int retval = -1, is_mpath = 0;
>-    char *map_device = NULL;
>+    int is_mpath = 0;
>     uint32_t minor = -1;
>     uint32_t next;
>+    VIR_AUTOFREE(char *) map_device = NULL;
>
>     do {
>         is_mpath = virStorageBackendIsMultipath(names->name);
>
>         if (is_mpath < 0)
>-            goto out;
>+            return -1;
>
>         if (is_mpath == 1) {
>
>             if (virAsprintf(&map_device, "mapper/%s", names->name) < 0)
>-                goto out;
>+                return -1;
>
>             if (virStorageBackendGetMinorNumber(names->name, &minor) < 0) {
>                 virReportError(VIR_ERR_INTERNAL_ERROR,
>                                _("Failed to get %s minor number"),
>                                names->name);
>-                goto out;
>+                return -1;
>             }
>
>             if (virStorageBackendMpathNewVol(pool, minor, map_device) < 0)
>-                goto out;
>+                return -1;
>
>-            VIR_FREE(map_device);

This is called in a loop, one VIR_FREE has to stay.

>         }
>
>         /* Given the way libdevmapper returns its data, I don't see
>diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
>index 14f01f9ec0..7460349c81 100644
>--- a/src/storage/storage_backend_scsi.c
>+++ b/src/storage/storage_backend_scsi.c
>@@ -56,16 +56,14 @@ static int
> virStorageBackendSCSITriggerRescan(uint32_t host)
> {
>     int fd = -1;
>-    int retval = 0;
>-    char *path;
>+    int retval = -1;

This inverts the logic of the function

>+    VIR_AUTOFREE(char *) path = NULL;
>
>     VIR_DEBUG("Triggering rescan of host %d", host);
>
>     if (virAsprintf(&path, "%s/host%u/scan",
>-                    LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
>-        retval = -1;
>-        goto out;
>-    }
>+                    LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
>+        return -1;
>
>     VIR_DEBUG("Scan trigger path is '%s'", path);
>
>@@ -75,8 +73,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>         virReportSystemError(errno,
>                              _("Could not open '%s' to trigger host scan"),
>                              path);

>-        retval = -1;
>-        goto free_path;
>+        goto cleanup;

Unrelated rename. (There's no jump to 'cleanup' with fd != -1)

>     }
>
>     if (safewrite(fd,
>@@ -86,13 +83,12 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>         virReportSystemError(errno,
>                              _("Write to '%s' to trigger host scan failed"),
>                              path);
>-        retval = -1;

Before, this returned -1, now it will return 0.

>     }
>
>+    retval = 0;
>+
>+ cleanup:
>     VIR_FORCE_CLOSE(fd);
>- free_path:
>-    VIR_FREE(path);
>- out:
>     VIR_DEBUG("Rescan of host %d complete", host);
>     return retval;
> }

>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 can also be separated into joining the success/error code paths and
actually switching to VIR_AUTOFREE.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190211/0b1d9dec/attachment-0001.sig>