[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