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@xxxxxxxxxx>
---
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
Attachment:
signature.asc
Description: PGP signature
