2:56 p.m.
- 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(a)redhat.com>
---
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 2db3f7d..3389edd 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4512,7 +4512,6 @@ testDestroyVport(testDriverPtr privconn,
const char *wwnn ATTRIBUTE_UNUSED,
const char *wwpn ATTRIBUTE_UNUSED)
{
- int ret = -1;
virNodeDeviceObjPtr obj = NULL;
virObjectEventPtr event = NULL;
@@ -4526,7 +4525,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",
@@ -4535,13 +4534,8 @@ testDestroyVport(testDriverPtr privconn,
virNodeDeviceObjRemove(&privconn->devs, &obj);
- ret = 0;
-
- cleanup:
- if (obj)
- virNodeDeviceObjUnlock(obj);
testObjectEventQueue(privconn, event);
- return ret;
+ return 0;
}
@@ -5328,16 +5322,14 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
virNodeDevicePtr ret = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, name)))
- goto cleanup;
+ return NULL;
if ((ret = virGetNodeDevice(conn, name))) {
if (VIR_STRDUP(ret->parent, obj->def->parent) < 0)
virObjectUnref(ret);
}
- cleanup:
- if (obj)
- virNodeDeviceObjUnlock(obj);
+ virNodeDeviceObjUnlock(obj);
return ret;
}
@@ -5352,13 +5344,11 @@ testNodeDeviceGetXMLDesc(virNodeDevicePtr dev,
virCheckFlags(0, NULL);
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
- goto cleanup;
+ return NULL;
ret = virNodeDeviceDefFormat(obj->def);
- cleanup:
- if (obj)
- virNodeDeviceObjUnlock(obj);
+ virNodeDeviceObjUnlock(obj);
return ret;
}
@@ -5370,7 +5360,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
char *ret = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
- goto cleanup;
+ return NULL;
if (obj->def->parent) {
ignore_value(VIR_STRDUP(ret, obj->def->parent));
@@ -5379,9 +5369,7 @@ testNodeDeviceGetParent(virNodeDevicePtr dev)
"%s", _("no parent for this device"));
}
- cleanup:
- if (obj)
- virNodeDeviceObjUnlock(obj);
+ virNodeDeviceObjUnlock(obj);
return ret;
}
@@ -5393,19 +5381,15 @@ testNodeDeviceNumOfCaps(virNodeDevicePtr dev)
virNodeDeviceObjPtr obj;
virNodeDevCapsDefPtr caps;
int ncaps = 0;
- int ret = -1;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
- goto cleanup;
+ return -1;
for (caps = obj->def->caps; caps; caps = caps->next)
++ncaps;
- ret = ncaps;
- cleanup:
- if (obj)
- virNodeDeviceObjUnlock(obj);
- return ret;
+ virNodeDeviceObjUnlock(obj);
+ return ncaps;
}
@@ -5416,26 +5400,25 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names,
int maxnames)
virNodeDeviceObjPtr obj;
virNodeDevCapsDefPtr caps;
int ncaps = 0;
- int ret = -1;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
- goto cleanup;
+ return -1;
for (caps = obj->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;
}
@@ -5584,13 +5567,13 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
virObjectEventPtr event = NULL;
if (!(obj = testNodeDeviceObjFindByName(driver, dev->name)))
- goto out;
+ return -1;
if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) == -1)
- goto out;
+ goto cleanup;
if (VIR_STRDUP(parent_name, obj->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
@@ -5603,7 +5586,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
if (virNodeDeviceObjGetParentHost(&driver->devs, obj->def,
EXISTING_DEVICE) < 0) {
obj = NULL;
- goto out;
+ goto cleanup;
}
event = virNodeDeviceEventLifecycleNew(dev->name,
@@ -5613,7 +5596,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
virNodeDeviceObjLock(obj);
virNodeDeviceObjRemove(&driver->devs, &obj);
- out:
+ cleanup:
if (obj)
virNodeDeviceObjUnlock(obj);
testObjectEventQueue(driver, event);
--
2.9.4