On 2/11/19 7:52 AM, Ján Tomko wrote:
> 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(-)
>>
[...]
>> 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;
>> }
>
So two pre-patches are attached with any luck...
John
>From 46e3b6fe94d68220c52e23e359d5b43a3f5cfbba Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@xxxxxxxxxx>
Date: Mon, 11 Feb 2019 21:48:53 -0500
Subject: [PATCH 2/2] storage: Invert retval logic in
virStorageBackendSCSITriggerRescan
Rather than initialize to 0 and change to -1 on error, let's do the
normal operation of initializing to -1 and set to 0 on success.
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/storage/storage_backend_scsi.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 85a177865f..591bcb04e2 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;
+ int retval = -1;
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;
+ LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
goto cleanup;
- }
VIR_DEBUG("Scan trigger path is '%s'", path);
@@ -75,7 +73,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
virReportSystemError(errno,
_("Could not open '%s' to trigger host scan"),
path);
- retval = -1;
goto cleanup;
}
@@ -85,9 +82,11 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
virReportSystemError(errno,
_("Write to '%s' to trigger host scan failed"),
path);
- retval = -1;
+ goto cleanup;
}
+ retval = 0;
+
cleanup:
VIR_FORCE_CLOSE(fd);
VIR_FREE(path);
--
2.20.1
>From e7e2aa6a7fb00a1207e9a5a52596fb4ec3ffce8b Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@xxxxxxxxxx>
Date: Mon, 11 Feb 2019 21:46:28 -0500
Subject: [PATCH 1/2] src: Fix label logic in
virStorageBackendSCSITriggerRescan
Let's initialize @path to NULL, then rather than use two labels
free_path and out labels, let's use the cleanup: label to call
VIR_FREE(path); and VIR_FORCE_CLOSE(fd);
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/storage/storage_backend_scsi.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 14f01f9ec0..85a177865f 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -57,14 +57,14 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
{
int fd = -1;
int retval = 0;
- char *path;
+ 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;
+ goto cleanup;
}
VIR_DEBUG("Scan trigger path is '%s'", path);
@@ -76,23 +76,21 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
_("Could not open '%s' to trigger host scan"),
path);
retval = -1;
- goto free_path;
+ goto cleanup;
}
if (safewrite(fd,
LINUX_SYSFS_SCSI_HOST_SCAN_STRING,
sizeof(LINUX_SYSFS_SCSI_HOST_SCAN_STRING)) < 0) {
- VIR_FORCE_CLOSE(fd);
virReportSystemError(errno,
_("Write to '%s' to trigger host scan failed"),
path);
retval = -1;
}
+ cleanup:
VIR_FORCE_CLOSE(fd);
- free_path:
VIR_FREE(path);
- out:
VIR_DEBUG("Rescan of host %d complete", host);
return retval;
}
--
2.20.1
