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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
  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@xxxxxxxxx>
- 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;
  }
[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux