[PATCH 12/12] remove unnecessary cleanup labels and unused return variables

Daniel Henrique Barboza danielhb413 at gmail.com
Wed Oct 28 11:23:55 UTC 2020



On 10/27/20 10:35 PM, Laine Stump wrote:
> After converting all DIR* to g_autoptr(DIR), many cleanup: labels
> ended up just having "return ret", and every place that set ret would
> just immediately goto cleanup. Remove the cleanup label and its
> return, and just return the set value immediately, thus eliminating
> the need for the return variable itself.
> 
> Signed-off-by: Laine Stump <laine at redhat.com>
> ---
>   src/conf/virnetworkobj.c       | 32 +++++++---------------
>   src/qemu/qemu_interop_config.c | 11 +++-----
>   src/storage/storage_util.c     | 50 +++++++++++++---------------------
>   src/util/vircgroup.c           | 16 ++++-------
>   src/util/vircommand.c          |  9 ++----
>   src/util/virdevmapper.c        |  6 ++--
>   src/util/virfile.c             | 45 ++++++++++++------------------
>   src/util/virnetdev.c           | 10 ++-----
>   src/util/virnuma.c             | 21 ++++++--------
>   src/util/virpci.c              | 27 ++++++------------
>   src/util/virresctrl.c          | 21 ++++++--------
>   src/util/virscsi.c             | 26 ++++++------------
>   src/util/virutil.c             | 20 ++++----------
>   src/util/virvhba.c             | 11 +++-----
>   tests/testutilsqemu.c          | 28 +++++++------------
>   15 files changed, 119 insertions(+), 214 deletions(-)
> 

[...]

>   
>   
> diff --git a/src/util/virscsi.c b/src/util/virscsi.c
> index 256acc37fa..22d4679368 100644
> --- a/src/util/virscsi.c
> +++ b/src/util/virscsi.c
> @@ -109,7 +109,6 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
>       g_autoptr(DIR) dir = NULL;
>       struct dirent *entry;
>       g_autofree char *path = NULL;
> -    char *sg = NULL;
>       unsigned int adapter_id;
>       const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_DEVICES;
>   
> @@ -120,16 +119,13 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
>                              bus, target, unit);
>   
>       if (virDirOpen(&dir, path) < 0)
> -        goto cleanup;
> +        return NULL;
>   
> -    while (virDirRead(dir, &entry, path) > 0) {
> -        /* Assume a single directory entry */
> -        sg = g_strdup(entry->d_name);
> -        break;
> -    }
> +    /* Assume a single directory entry */
> +    if (virDirRead(dir, &entry, path) > 0)
> +        return  g_strdup(entry->d_name);


Nit: extra whitespace after 'return'


Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>

>   
> - cleanup:
> -    return sg;
> +    return NULL;
>   }
>   
>   /* Returns device name (e.g. "sdc") on success, or NULL
> @@ -145,7 +141,6 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
>       g_autoptr(DIR) dir = NULL;
>       struct dirent *entry;
>       g_autofree char *path = NULL;
> -    char *name = NULL;
>       unsigned int adapter_id;
>       const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_DEVICES;
>   
> @@ -156,15 +151,12 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
>                              target, unit);
>   
>       if (virDirOpen(&dir, path) < 0)
> -        goto cleanup;
> +        return NULL;
>   
> -    while (virDirRead(dir, &entry, path) > 0) {
> -        name = g_strdup(entry->d_name);
> -        break;
> -    }
> +    if (virDirRead(dir, &entry, path) > 0)
> +        return g_strdup(entry->d_name);
>   
> - cleanup:
> -    return name;
> +    return NULL;
>   }
>   
>   virSCSIDevicePtr
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 41e92023fc..a0cd0f1bcd 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -1623,22 +1623,18 @@ virHostHasIOMMU(void)
>   {
>       g_autoptr(DIR) iommuDir = NULL;
>       struct dirent *iommuGroup = NULL;
> -    bool ret = false;
>       int direrr;
>   
>       if (virDirOpenQuiet(&iommuDir, "/sys/kernel/iommu_groups/") < 0)
> -        goto cleanup;
> +        return false;
>   
>       while ((direrr = virDirRead(iommuDir, &iommuGroup, NULL)) > 0)
>           break;
>   
>       if (direrr < 0 || !iommuGroup)
> -        goto cleanup;
> -
> -    ret = true;
> +        return false;
>   
> - cleanup:
> -    return ret;
> +    return true;
>   }
>   
>   
> @@ -1656,7 +1652,6 @@ virHostHasIOMMU(void)
>   char *
>   virHostGetDRMRenderNode(void)
>   {
> -    char *ret = NULL;
>       g_autoptr(DIR) driDir = NULL;
>       const char *driPath = "/dev/dri";
>       struct dirent *ent = NULL;
> @@ -1674,19 +1669,16 @@ virHostGetDRMRenderNode(void)
>       }
>   
>       if (dirErr < 0)
> -        goto cleanup;
> +        return NULL;
>   
>       /* even if /dev/dri exists, there might be no renderDX nodes available */
>       if (!have_rendernode) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          _("No DRM render nodes available"));
> -        goto cleanup;
> +        return NULL;
>       }
>   
> -    ret = g_strdup_printf("%s/%s", driPath, ent->d_name);
> -
> - cleanup:
> -    return ret;
> +    return g_strdup_printf("%s/%s", driPath, ent->d_name);
>   }
>   
>   
> diff --git a/src/util/virvhba.c b/src/util/virvhba.c
> index 471d94d3dd..a4e88024d1 100644
> --- a/src/util/virvhba.c
> +++ b/src/util/virvhba.c
> @@ -365,7 +365,6 @@ virVHBAGetHostByWWN(const char *sysfs_prefix,
>       const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH;
>       struct dirent *entry = NULL;
>       g_autoptr(DIR) dir = NULL;
> -    char *ret = NULL;
>   
>       if (virDirOpen(&dir, prefix) < 0)
>           return NULL;
> @@ -375,24 +374,22 @@ virVHBAGetHostByWWN(const char *sysfs_prefix,
>   
>           if ((rc = vhbaReadCompareWWN(prefix, entry->d_name,
>                                        "node_name", wwnn)) < 0)
> -            goto cleanup;
> +            return NULL;
>   
>           if (rc == 0)
>               continue;
>   
>           if ((rc = vhbaReadCompareWWN(prefix, entry->d_name,
>                                        "port_name", wwpn)) < 0)
> -            goto cleanup;
> +            return NULL;
>   
>           if (rc == 0)
>               continue;
>   
> -        ret = g_strdup(entry->d_name);
> -        break;
> +        return g_strdup(entry->d_name);
>       }
>   
> - cleanup:
> -    return ret;
> +    return NULL;
>   }
>   
>   /* virVHBAGetHostByFabricWWN:
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index 5ae1d64337..c6848c12a2 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -516,12 +516,11 @@ testQemuGetLatestCapsForArch(const char *arch,
>       unsigned long maxver = 0;
>       unsigned long ver;
>       g_autofree char *maxname = NULL;
> -    char *ret = NULL;
>   
>       fullsuffix = g_strdup_printf("%s.%s", arch, suffix);
>   
>       if (virDirOpen(&dir, TEST_QEMU_CAPS_PATH) < 0)
> -        goto cleanup;
> +        return NULL;
>   
>       while ((rc = virDirRead(dir, &ent, TEST_QEMU_CAPS_PATH)) > 0) {
>           g_autofree char *tmp = NULL;
> @@ -547,18 +546,15 @@ testQemuGetLatestCapsForArch(const char *arch,
>       }
>   
>       if (rc < 0)
> -        goto cleanup;
> +        return NULL;
>   
>       if (!maxname) {
>           VIR_TEST_VERBOSE("failed to find capabilities for '%s' in '%s'",
>                            arch, TEST_QEMU_CAPS_PATH);
> -        goto cleanup;
> +        return NULL;
>       }
>   
> -    ret = g_strdup_printf("%s/%s", TEST_QEMU_CAPS_PATH, maxname);
> -
> - cleanup:
> -    return ret;
> +    return g_strdup_printf("%s/%s", TEST_QEMU_CAPS_PATH, maxname);
>   }
>   
>   
> @@ -607,7 +603,6 @@ testQemuCapsIterate(const char *suffix,
>       struct dirent *ent;
>       g_autoptr(DIR) dir = NULL;
>       int rc;
> -    int ret = -1;
>       bool fail = false;
>   
>       if (!callback)
> @@ -616,11 +611,11 @@ testQemuCapsIterate(const char *suffix,
>       /* Validate suffix */
>       if (!STRPREFIX(suffix, ".")) {
>           VIR_TEST_VERBOSE("malformed suffix '%s'", suffix);
> -        goto cleanup;
> +        return -1;
>       }
>   
>       if (virDirOpen(&dir, TEST_QEMU_CAPS_PATH) < 0)
> -        goto cleanup;
> +        return -1;
>   
>       while ((rc = virDirRead(dir, &ent, TEST_QEMU_CAPS_PATH)) > 0) {
>           g_autofree char *tmp = g_strdup(ent->d_name);
> @@ -634,13 +629,13 @@ testQemuCapsIterate(const char *suffix,
>           /* Strip the leading prefix */
>           if (!(version = STRSKIP(tmp, "caps_"))) {
>               VIR_TEST_VERBOSE("malformed file name '%s'", ent->d_name);
> -            goto cleanup;
> +            return -1;
>           }
>   
>           /* Find the last dot */
>           if (!(archName = strrchr(tmp, '.'))) {
>               VIR_TEST_VERBOSE("malformed file name '%s'", ent->d_name);
> -            goto cleanup;
> +            return -1;
>           }
>   
>           /* The version number and the architecture name are separated by
> @@ -661,12 +656,9 @@ testQemuCapsIterate(const char *suffix,
>       }
>   
>       if (rc < 0 || fail)
> -        goto cleanup;
> -
> -    ret = 0;
> +        return -1;
>   
> - cleanup:
> -    return ret;
> +    return 0;
>   }
>   
>   
> 




More information about the libvir-list mailing list