[PATCH v3 02/12] test: Adjust cleanup/error paths for nodedev test APIs

[John Ferlan <jferlan@xxxxxxxxxx>]

 - Rather than "goto cleanup;" on failure to virNodeDeviceObjFindByName
   an @obj, just return directly.  This then allows the cleanup: label code
   to not have to check "if (obj)" before calling virNodeDeviceObjUnlock.
   This also simplifies some exit logic...

 - In testNodeDeviceObjFindByName use an error: label to handle the failure
   and don't do the ncaps++ within the VIR_STRDUP() source target index.
   Only increment ncaps after success. Easier on eyes at error label too.

 - In testNodeDeviceDestroy use "cleanup" rather than "out" for the goto

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/test/test_driver.c | 75 +++++++++++++++++++-------------------------------
 1 file changed, 29 insertions(+), 46 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e323619..e91dfa3 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4509,7 +4509,6 @@ testDestroyVport(testDriverPtr privconn,
                  const char *wwnn ATTRIBUTE_UNUSED,
                  const char *wwpn ATTRIBUTE_UNUSED)
 {
-    int ret = -1;
     virNodeDeviceObjPtr obj = NULL;
     virObjectEventPtr event = NULL;
 
@@ -4523,7 +4522,7 @@ testDestroyVport(testDriverPtr privconn,
     if (!(obj = virNodeDeviceObjFindByName(&privconn->devs, "scsi_host12"))) {
         virReportError(VIR_ERR_NO_NODE_DEVICE, "%s",
                        _("no node device with matching name 'scsi_host12'"));
-        goto cleanup;
+        return -1;
     }
 
     event = virNodeDeviceEventLifecycleNew("scsi_host12",
@@ -4534,13 +4533,8 @@ testDestroyVport(testDriverPtr privconn,
     virNodeDeviceObjFree(obj);
     obj = NULL;
 
-    ret = 0;
-
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
     testObjectEventQueue(privconn, event);
-    return ret;
+    return 0;
 }
 
 
@@ -5328,7 +5322,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
     virNodeDevicePtr ret = NULL;
 
     if (!(obj = testNodeDeviceObjFindByName(driver, name)))
-        goto cleanup;
+        return NULL;
     def = virNodeDeviceObjGetDef(obj);
 
     if ((ret = virGetNodeDevice(conn, name))) {
@@ -5338,9 +5332,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
         }
     }
 
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjUnlock(obj);
     return ret;
 }
 
@@ -5355,13 +5347,11 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev,
     virCheckFlags(0, NULL);
 
     if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
-        goto cleanup;
+        return NULL;
 
     ret = virNodeDeviceDefFormat(virNodeDeviceObjGetDef(obj));
 
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjUnlock(obj);
     return ret;
 }
 
@@ -5374,7 +5364,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
     char *ret = NULL;
 
     if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
-        goto cleanup;
+        return NULL;
     def = virNodeDeviceObjGetDef(obj);
 
     if (def->parent) {
@@ -5384,9 +5374,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
                        "%s", _("no parent for this device"));
     }
 
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjUnlock(obj);
     return ret;
 }
 
@@ -5399,20 +5387,16 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev)
     virNodeDeviceDefPtr def;
     virNodeDevCapsDefPtr caps;
     int ncaps = 0;
-    int ret = -1;
 
     if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
-        goto cleanup;
+        return -1;
     def = virNodeDeviceObjGetDef(obj);
 
     for (caps = def->caps; caps; caps = caps->next)
         ++ncaps;
-    ret = ncaps;
 
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
-    return ret;
+    virNodeDeviceObjUnlock(obj);
+    return ncaps;
 }
 
 
@@ -5424,27 +5408,26 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames)
     virNodeDeviceDefPtr def;
     virNodeDevCapsDefPtr caps;
     int ncaps = 0;
-    int ret = -1;
 
     if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
-        goto cleanup;
+        return -1;
     def = virNodeDeviceObjGetDef(obj);
 
     for (caps = def->caps; caps && ncaps < maxnames; caps = caps->next) {
-        if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->data.type)) < 0)
-            goto cleanup;
+        if (VIR_STRDUP(names[ncaps],
+                       virNodeDevCapTypeToString(caps->data.type)) < 0)
+            goto error;
+        ncaps++;
     }
-    ret = ncaps;
 
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
-    if (ret == -1) {
-        --ncaps;
-        while (--ncaps >= 0)
-            VIR_FREE(names[ncaps]);
-    }
-    return ret;
+    virNodeDeviceObjUnlock(obj);
+    return ncaps;
+
+ error:
+    while (--ncaps >= 0)
+        VIR_FREE(names[ncaps]);
+    virNodeDeviceObjUnlock(obj);
+    return -1;
 }
 
 
@@ -5598,14 +5581,14 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
     virObjectEventPtr event = NULL;
 
     if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
-        goto out;
+        return -1;
     def = virNodeDeviceObjGetDef(obj);
 
     if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
-        goto out;
+        goto cleanup;
 
     if (VIR_STRDUP(parent_name, def->parent) < 0)
-        goto out;
+        goto cleanup;
 
     /* virNodeDeviceGetParentHost will cause the device object's lock to be
      * taken, so we have to dup the parent's name and drop the lock
@@ -5618,7 +5601,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
     if (virNodeDeviceObjGetParentHost(&driver->devs, def,
                                       EXISTING_DEVICE) < 0) {
         obj = NULL;
-        goto out;
+        goto cleanup;
     }
 
     event = virNodeDeviceEventLifecycleNew(dev->name,
@@ -5630,7 +5613,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
     virNodeDeviceObjFree(obj);
     obj = NULL;
 
- out:
+ cleanup:
     if (obj)
         virNodeDeviceObjUnlock(obj);
     testObjectEventQueue(driver, event);
-- 
2.9.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list