test: Adjust cleanup/error paths for nodedev test APIs

 - In testDestroyVport rather than use a cleanup label, just return -1
   immediately since nothing else is needed.

 - In testStoragePoolDestroy, if !privpool, then just return -1 since
   nothing else will happen anyway.

 - 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@redhat.com>

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e17c86a1e5a2d31a426bf16ce420a9b011162d30..bd24e5ba9debdbc2eb641e2398932e06fda9e205 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4550,7 +4550,6 @@ testDestroyVport(testDriverPtr privconn,
                  const char *wwnn ATTRIBUTE_UNUSED,
                  const char *wwpn ATTRIBUTE_UNUSED)
 {
-    int ret = -1;
     virNodeDeviceObjPtr obj = NULL;
     virObjectEventPtr event = NULL;
 
@@ -4564,7 +4563,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",
@@ -4573,15 +4572,9 @@ testDestroyVport(testDriverPtr privconn,
 
     virNodeDeviceObjRemove(&privconn->devs, obj);
     virNodeDeviceObjFree(obj);
-    obj = NULL;
 
-    ret = 0;
-
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
     testObjectEventQueue(privconn, event);
-    return ret;
+    return 0;
 }
 
 
@@ -4594,7 +4587,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
     virObjectEventPtr event = NULL;
 
     if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name)))
-        goto cleanup;
+        return -1;
 
     if (!virStoragePoolObjIsActive(privpool)) {
         virReportError(VIR_ERR_OPERATION_INVALID,
@@ -5369,7 +5362,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))) {
@@ -5379,9 +5372,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
         }
     }
 
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjUnlock(obj);
     return ret;
 }
 
@@ -5396,13 +5387,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;
 }
 
@@ -5415,7 +5404,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
     char *ret = NULL;
 
     if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
-        goto cleanup;
+        return NULL;
     def = virNodeDeviceObjGetDef(obj);
 
     if (def->parent) {
@@ -5425,9 +5414,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
                        "%s", _("no parent for this device"));
     }
 
- cleanup:
-    if (obj)
-        virNodeDeviceObjUnlock(obj);
+    virNodeDeviceObjUnlock(obj);
     return ret;
 }
 
@@ -5440,20 +5427,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;
 }
 
 
@@ -5465,27 +5448,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;
 }
 
 
@@ -5639,14 +5621,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
@@ -5659,7 +5641,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
     if (virNodeDeviceObjGetParentHost(&driver->devs, def,
                                       EXISTING_DEVICE) < 0) {
         obj = NULL;
-        goto out;
+        goto cleanup;
     }
 
     event = virNodeDeviceEventLifecycleNew(dev->name,
@@ -5671,7 +5653,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
     virNodeDeviceObjFree(obj);
     obj = NULL;
 
- out:
+ cleanup:
     if (obj)
         virNodeDeviceObjUnlock(obj);
     testObjectEventQueue(driver, event);