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

> Just a minor nit, while I was checking where testDestroyVport gets called from > - testStoragePoolDestroy, there's a spot that could be fixed the same way. > Would you mind squashing this bit in as well? > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > index 6422ba8fb..a397364c7 100644 > --- a/src/test/test_driver.c > +++ b/src/test/test_driver.c > @@ -4547,7 +4547,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool) > virObjectEventPtr event = NULL; > > if (!(privpool = testStoragePoolObjFindByName(privconn, pool->name))) > - goto cleanup; > + return -1; > I don't mind changing this as well - although I probably have that in another patch somewhere dealing with storage tests. Trying to keep nodedev with nodedev and storage with storage.

> if (!virStoragePoolObjIsActive(privpool)) { > virReportError(VIR_ERR_OPERATION_INVALID, > > > > [...] > >> @@ -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++; > > How about placing the increment to the iteration_expression segment of the > loop, aka at the end where the usual increment happens before condition > evaluation. Wondering whether there's a general technical term for the syntax > part of the loop between the parentheses. > Do you mean : (caps = def->caps; caps && ncaps < maxnames; caps = caps->next, ncaps++) ?? If so, yuck. Since we're iterating on caps, I think what should be incremented is caps and not ncaps as well. Putting it after the