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

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(a)redhat.com&gt; > --- > 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; > }