Re: [libvirt] [PATCH 7/8] interface: Clean up virInterfaceObjListFindByMACString

On 04/26/2017 12:36 AM, John Ferlan wrote: > Alter the algorithm to return a list of matching names rather than a > list of match virInterfaceObjPtr which are then just dereferenced > extracting the def->name and def->mac. Since the def->mac would be > the same as the passed @mac, just return a list of names and as long > as there's only one, extract the [0] entry from the passed list. > Also alter the error message on failure to include the mac that wasn't > found. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/conf/virinterfaceobj.c | 23 ++++++++++++++--------- > src/conf/virinterfaceobj.h | 2 +- > src/test/test_driver.c | 16 ++++++++-------- > 3 files changed, 23 insertions(+), 18 deletions(-) > > diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c > index 3aeeebd..1cc5c92 100644 > --- a/src/conf/virinterfaceobj.c > +++ b/src/conf/virinterfaceobj.c > @@ -108,11 +108,11 @@ virInterfaceObjListNew(void) > int > virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces, > const char *mac, > - virInterfaceObjPtr *matches, > + char **const matches, > int maxmatches) > { > size_t i; > - unsigned int matchct = 0; > + int matchct = 0; > > for (i = 0; i < interfaces->count; i++) { > virInterfaceObjPtr obj = interfaces->objs[i]; > @@ -121,18 +121,23 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces, > virInterfaceObjLock(obj); > def = obj->def; > if (STRCASEEQ(def->mac, mac)) { > - matchct++; > - if (matchct <= maxmatches) { > - matches[matchct - 1] = obj; > - /* keep the lock if we're returning object to caller */ > - /* it is the caller's responsibility to unlock *all* matches */ > - continue; > + if (matchct < maxmatches) { > + if (VIR_STRDUP(matches[matchct], def->name) < 0) { > + virInterfaceObjUnlock(obj); > + goto error; > + } > + matchct++; > } > } > virInterfaceObjUnlock(obj); > - > } > return matchct; > + > + error: > + while (--matchct >= 0) > + VIR_FREE(matches[matchct]); > + > + return -1; > } > > > diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h > index f1bcab9..3934e63 100644 > --- a/src/conf/virinterfaceobj.h > +++ b/src/conf/virinterfaceobj.h > @@ -44,7 +44,7 @@ virInterfaceObjListNew(void); > int > virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces, > const char *mac, > - virInterfaceObjPtr *matches, > + char **const matches, > int maxmatches); > > virInterfaceObjPtr > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > index 89a705c..ac16f4f 100644 > --- a/src/test/test_driver.c > +++ b/src/test/test_driver.c > @@ -3728,17 +3728,18 @@ testInterfaceLookupByMACString(virConnectPtr conn, > const char *mac) > { > testDriverPtr privconn = conn->privateData; > - virInterfaceObjPtr obj; > - virInterfaceDefPtr def; > int ifacect; > + char *ifacenames[] = { NULL, NULL }; > virInterfacePtr ret = NULL; > > testDriverLock(privconn); > - ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, &obj, 1); > + ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, > + ifacenames, 2); ARRAY_CARDINALITY()

> testDriverUnlock(privconn); > > if (ifacect == 0) { > - virReportError(VIR_ERR_NO_INTERFACE, NULL); > + virReportError(VIR_ERR_NO_INTERFACE, > + _("no interface with matching mac '%s'"), mac); > goto cleanup; > } > > @@ -3747,12 +3748,11 @@ testInterfaceLookupByMACString(virConnectPtr conn, > goto cleanup; > } > > - def = virInterfaceObjGetDef(obj); > - ret = virGetInterface(conn, def->name, def->mac); > + ret = virGetInterface(conn, ifacenames[0], mac); > > cleanup: > - if (obj) > - virInterfaceObjUnlock(obj); > + VIR_FREE(ifacenames[0]); > + VIR_FREE(ifacenames[1]); And this can be probably dynamic then: for (i = 0; i < ARRAY_CARDINALITY(ifacenames); i++) VIR_FREE(ifacenames[i]); > return ret; > } > > Michal