Alter variable names to be obj rather than 'iface' and/or 'obj'. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virinterfaceobj.c | 48 ++++++++++++++-------------- src/conf/virinterfaceobj.h | 4 +-- src/test/test_driver.c | 78 +++++++++++++++++++++++----------------------- 3 files changed, 65 insertions(+), 65 deletions(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index e80db23..62c3735 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -51,14 +51,14 @@ virInterfaceObjUnlock(virInterfaceObjPtr obj) void -virInterfaceObjFree(virInterfaceObjPtr iface) +virInterfaceObjFree(virInterfaceObjPtr obj) { - if (!iface) + if (!obj) return; - virInterfaceDefFree(iface->def); - virMutexDestroy(&iface->lock); - VIR_FREE(iface); + virInterfaceDefFree(obj->def); + virMutexDestroy(&obj->lock); + VIR_FREE(obj); } @@ -136,7 +136,7 @@ virInterfaceObjListClone(virInterfaceObjListPtr src, for (i = 0; i < cnt; i++) { virInterfaceDefPtr def = src->objs[i]->def; virInterfaceDefPtr backup; - virInterfaceObjPtr iface; + virInterfaceObjPtr obj; char *xml = virInterfaceDefFormat(def); if (!xml) @@ -148,9 +148,9 @@ virInterfaceObjListClone(virInterfaceObjListPtr src, } VIR_FREE(xml); - if ((iface = virInterfaceObjAssignDef(dest, backup)) == NULL) + if ((obj = virInterfaceObjAssignDef(dest, backup)) == NULL) goto cleanup; - virInterfaceObjUnlock(iface); /* locked by virInterfaceObjAssignDef */ + virInterfaceObjUnlock(obj); /* locked by virInterfaceObjAssignDef */ } ret = cnt; @@ -165,47 +165,47 @@ virInterfaceObjPtr virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces, virInterfaceDefPtr def) { - virInterfaceObjPtr iface; + virInterfaceObjPtr obj; - if ((iface = virInterfaceObjFindByName(interfaces, def->name))) { - virInterfaceDefFree(iface->def); - iface->def = def; + if ((obj = virInterfaceObjFindByName(interfaces, def->name))) { + virInterfaceDefFree(obj->def); + obj->def = def; - return iface; + return obj; } - if (VIR_ALLOC(iface) < 0) + if (VIR_ALLOC(obj) < 0) return NULL; - if (virMutexInit(&iface->lock) < 0) { + if (virMutexInit(&obj->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize mutex")); - VIR_FREE(iface); + VIR_FREE(obj); return NULL; } - virInterfaceObjLock(iface); + virInterfaceObjLock(obj); if (VIR_APPEND_ELEMENT_COPY(interfaces->objs, - interfaces->count, iface) < 0) { - virInterfaceObjFree(iface); + interfaces->count, obj) < 0) { + virInterfaceObjFree(obj); return NULL; } - iface->def = def; - return iface; + obj->def = def; + return obj; } void virInterfaceObjRemove(virInterfaceObjListPtr interfaces, - virInterfaceObjPtr iface) + virInterfaceObjPtr obj) { size_t i; - virInterfaceObjUnlock(iface); + virInterfaceObjUnlock(obj); for (i = 0; i < interfaces->count; i++) { virInterfaceObjLock(interfaces->objs[i]); - if (interfaces->objs[i] == iface) { + if (interfaces->objs[i] == obj) { virInterfaceObjUnlock(interfaces->objs[i]); virInterfaceObjFree(interfaces->objs[i]); diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h index 5b0527d..ee166c6 100644 --- a/src/conf/virinterfaceobj.h +++ b/src/conf/virinterfaceobj.h @@ -54,7 +54,7 @@ virInterfaceObjFindByName(virInterfaceObjListPtr interfaces, const char *name); void -virInterfaceObjFree(virInterfaceObjPtr iface); +virInterfaceObjFree(virInterfaceObjPtr obj); void virInterfaceObjListFree(virInterfaceObjListPtr vms); @@ -69,7 +69,7 @@ virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces, void virInterfaceObjRemove(virInterfaceObjListPtr interfaces, - virInterfaceObjPtr iface); + virInterfaceObjPtr obj); void virInterfaceObjLock(virInterfaceObjPtr obj); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2db3f7d..c2697e8 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3627,18 +3627,18 @@ static virInterfaceObjPtr testInterfaceObjFindByName(testDriverPtr privconn, const char *name) { - virInterfaceObjPtr iface; + virInterfaceObjPtr obj; testDriverLock(privconn); - iface = virInterfaceObjFindByName(&privconn->ifaces, name); + obj = virInterfaceObjFindByName(&privconn->ifaces, name); testDriverUnlock(privconn); - if (!iface) + if (!obj) virReportError(VIR_ERR_NO_INTERFACE, _("no interface with matching name '%s'"), name); - return iface; + return obj; } @@ -3705,17 +3705,17 @@ testInterfaceLookupByName(virConnectPtr conn, const char *name) { testDriverPtr privconn = conn->privateData; - virInterfaceObjPtr iface; + virInterfaceObjPtr obj; virInterfacePtr ret = NULL; - if (!(iface = testInterfaceObjFindByName(privconn, name))) + if (!(obj = testInterfaceObjFindByName(privconn, name))) goto cleanup; - ret = virGetInterface(conn, iface->def->name, iface->def->mac); + ret = virGetInterface(conn, obj->def->name, obj->def->mac); cleanup: - if (iface) - virInterfaceObjUnlock(iface); + if (obj) + virInterfaceObjUnlock(obj); return ret; } @@ -3725,12 +3725,12 @@ testInterfaceLookupByMACString(virConnectPtr conn, const char *mac) { testDriverPtr privconn = conn->privateData; - virInterfaceObjPtr iface; + virInterfaceObjPtr obj; int ifacect; virInterfacePtr ret = NULL; testDriverLock(privconn); - ifacect = virInterfaceObjFindByMACString(&privconn->ifaces, mac, &iface, 1); + ifacect = virInterfaceObjFindByMACString(&privconn->ifaces, mac, &obj, 1); testDriverUnlock(privconn); if (ifacect == 0) { @@ -3743,11 +3743,11 @@ testInterfaceLookupByMACString(virConnectPtr conn, goto cleanup; } - ret = virGetInterface(conn, iface->def->name, iface->def->mac); + ret = virGetInterface(conn, obj->def->name, obj->def->mac); cleanup: - if (iface) - virInterfaceObjUnlock(iface); + if (obj) + virInterfaceObjUnlock(obj); return ret; } @@ -3869,19 +3869,19 @@ testInterfaceGetXMLDesc(virInterfacePtr iface, unsigned int flags) { testDriverPtr privconn = iface->conn->privateData; - virInterfaceObjPtr privinterface; + virInterfaceObjPtr obj; char *ret = NULL; virCheckFlags(0, NULL); - if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) + if (!(obj = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - ret = virInterfaceDefFormat(privinterface->def); + ret = virInterfaceDefFormat(obj->def); cleanup: - if (privinterface) - virInterfaceObjUnlock(privinterface); + if (obj) + virInterfaceObjUnlock(obj); return ret; } @@ -3893,7 +3893,7 @@ testInterfaceDefineXML(virConnectPtr conn, { testDriverPtr privconn = conn->privateData; virInterfaceDefPtr def; - virInterfaceObjPtr iface = NULL; + virInterfaceObjPtr obj = NULL; virInterfacePtr ret = NULL; virCheckFlags(0, NULL); @@ -3902,16 +3902,16 @@ testInterfaceDefineXML(virConnectPtr conn, if ((def = virInterfaceDefParseString(xmlStr)) == NULL) goto cleanup; - if ((iface = virInterfaceObjAssignDef(&privconn->ifaces, def)) == NULL) + if ((obj = virInterfaceObjAssignDef(&privconn->ifaces, def)) == NULL) goto cleanup; def = NULL; - ret = virGetInterface(conn, iface->def->name, iface->def->mac); + ret = virGetInterface(conn, obj->def->name, obj->def->mac); cleanup: virInterfaceDefFree(def); - if (iface) - virInterfaceObjUnlock(iface); + if (obj) + virInterfaceObjUnlock(obj); testDriverUnlock(privconn); return ret; } @@ -3921,13 +3921,13 @@ static int testInterfaceUndefine(virInterfacePtr iface) { testDriverPtr privconn = iface->conn->privateData; - virInterfaceObjPtr privinterface; + virInterfaceObjPtr obj; int ret = -1; - if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) + if (!(obj = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - virInterfaceObjRemove(&privconn->ifaces, privinterface); + virInterfaceObjRemove(&privconn->ifaces, obj); ret = 0; cleanup: @@ -3940,25 +3940,25 @@ testInterfaceCreate(virInterfacePtr iface, unsigned int flags) { testDriverPtr privconn = iface->conn->privateData; - virInterfaceObjPtr privinterface; + virInterfaceObjPtr obj; int ret = -1; virCheckFlags(0, -1); - if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) + if (!(obj = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - if (privinterface->active != 0) { + if (obj->active != 0) { virReportError(VIR_ERR_OPERATION_INVALID, NULL); goto cleanup; } - privinterface->active = 1; + obj->active = 1; ret = 0; cleanup: - if (privinterface) - virInterfaceObjUnlock(privinterface); + if (obj) + virInterfaceObjUnlock(obj); return ret; } @@ -3968,25 +3968,25 @@ testInterfaceDestroy(virInterfacePtr iface, unsigned int flags) { testDriverPtr privconn = iface->conn->privateData; - virInterfaceObjPtr privinterface; + virInterfaceObjPtr obj; int ret = -1; virCheckFlags(0, -1); - if (!(privinterface = testInterfaceObjFindByName(privconn, iface->name))) + if (!(obj = testInterfaceObjFindByName(privconn, iface->name))) goto cleanup; - if (privinterface->active == 0) { + if (obj->active == 0) { virReportError(VIR_ERR_OPERATION_INVALID, NULL); goto cleanup; } - privinterface->active = 0; + obj->active = 0; ret = 0; cleanup: - if (privinterface) - virInterfaceObjUnlock(privinterface); + if (obj) + virInterfaceObjUnlock(obj); return ret; } -- 2.9.4

On Fri, May 26, 2017 at 07:59:01 -0400, John Ferlan wrote: [...] This hunk does something that is not advertised in the commit message. ACK to the rest of the patch.

We're about to make the obj much more private, so make it easier to see future changes which will require accessors for the obj->def This also includes modifying some interfaces->objs[i]->X references to be obj = interfaces->objs[i]; and then def = obj->def Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virinterfaceobj.c | 32 +++++++++++++++++++++----------- src/test/test_driver.c | 12 +++++++++--- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index 62c3735..ead9512 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -72,18 +72,21 @@ virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces, unsigned int matchct = 0; for (i = 0; i < interfaces->count; i++) { + virInterfaceObjPtr obj = interfaces->objs[i]; + virInterfaceDefPtr def; - virInterfaceObjLock(interfaces->objs[i]); - if (STRCASEEQ(interfaces->objs[i]->def->mac, mac)) { + virInterfaceObjLock(obj); + def = obj->def; + if (STRCASEEQ(def->mac, mac)) { matchct++; if (matchct <= maxmatches) { - matches[matchct - 1] = interfaces->objs[i]; + 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; } } - virInterfaceObjUnlock(interfaces->objs[i]); + virInterfaceObjUnlock(obj); } return matchct; @@ -97,10 +100,14 @@ virInterfaceObjFindByName(virInterfaceObjListPtr interfaces, size_t i; for (i = 0; i < interfaces->count; i++) { - virInterfaceObjLock(interfaces->objs[i]); - if (STREQ(interfaces->objs[i]->def->name, name)) - return interfaces->objs[i]; - virInterfaceObjUnlock(interfaces->objs[i]); + virInterfaceObjPtr obj = interfaces->objs[i]; + virInterfaceDefPtr def; + + virInterfaceObjLock(obj); + def = obj->def; + if (STREQ(def->name, name)) + return obj; + virInterfaceObjUnlock(obj); } return NULL; @@ -134,10 +141,10 @@ virInterfaceObjListClone(virInterfaceObjListPtr src, virInterfaceObjListFree(dest); /* start with an empty list */ cnt = src->count; for (i = 0; i < cnt; i++) { - virInterfaceDefPtr def = src->objs[i]->def; + virInterfaceObjPtr srcobj = src->objs[i]; virInterfaceDefPtr backup; virInterfaceObjPtr obj; - char *xml = virInterfaceDefFormat(def); + char *xml = virInterfaceDefFormat(srcobj->def); if (!xml) goto cleanup; @@ -247,9 +254,12 @@ virInterfaceObjGetNames(virInterfaceObjListPtr interfaces, for (i = 0; i < interfaces->count && nnames < maxnames; i++) { virInterfaceObjPtr obj = interfaces->objs[i]; + virInterfaceDefPtr def; + virInterfaceObjLock(obj); + def = obj->def; if (wantActive == virInterfaceObjIsActive(obj)) { - if (VIR_STRDUP(names[nnames], obj->def->name) < 0) { + if (VIR_STRDUP(names[nnames], def->name) < 0) { virInterfaceObjUnlock(obj); goto failure; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index da45542..6ff4657 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3706,12 +3706,14 @@ testInterfaceLookupByName(virConnectPtr conn, { testDriverPtr privconn = conn->privateData; virInterfaceObjPtr obj; + virInterfaceDefPtr def; virInterfacePtr ret = NULL; if (!(obj = testInterfaceObjFindByName(privconn, name))) return NULL; + def = obj->def; - ret = virGetInterface(conn, obj->def->name, obj->def->mac); + ret = virGetInterface(conn, def->name, def->mac); virInterfaceObjUnlock(obj); return ret; @@ -3724,6 +3726,7 @@ testInterfaceLookupByMACString(virConnectPtr conn, { testDriverPtr privconn = conn->privateData; virInterfaceObjPtr obj; + virInterfaceDefPtr def; int ifacect; virInterfacePtr ret = NULL; @@ -3741,7 +3744,8 @@ testInterfaceLookupByMACString(virConnectPtr conn, goto cleanup; } - ret = virGetInterface(conn, obj->def->name, obj->def->mac); + def = obj->def; + ret = virGetInterface(conn, def->name, def->mac); cleanup: if (obj) @@ -3888,6 +3892,7 @@ testInterfaceDefineXML(virConnectPtr conn, testDriverPtr privconn = conn->privateData; virInterfaceDefPtr def; virInterfaceObjPtr obj = NULL; + virInterfaceDefPtr objdef; virInterfacePtr ret = NULL; virCheckFlags(0, NULL); @@ -3899,8 +3904,9 @@ testInterfaceDefineXML(virConnectPtr conn, if ((obj = virInterfaceObjAssignDef(&privconn->ifaces, def)) == NULL) goto cleanup; def = NULL; + objdef = obj->def; - ret = virGetInterface(conn, obj->def->name, obj->def->mac); + ret = virGetInterface(conn, objdef->name, objdef->mac); cleanup: virInterfaceDefFree(def); -- 2.9.4

Move the structs into virinterfaceobj.c, create necessary accessors, and initializers. This also includes reworking virInterfaceObjListClone to handle receiving a source interfaces list pointer, creating the destination interfaces object, and copying everything from source into dest. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virinterfaceobj.c | 57 +++++++++++++++++++++++++++++----------------- src/conf/virinterfaceobj.h | 12 ++++------ src/libvirt_private.syms | 1 + src/test/test_driver.c | 38 +++++++++++++++---------------- 4 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index a2ef7f4..dd86151 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -39,6 +39,10 @@ struct _virInterfaceObj { virInterfaceDefPtr def; /* The interface definition */ }; +struct _virInterfaceObjList { + size_t count; + virInterfaceObjPtr *objs; +}; /* virInterfaceObj manipulation */ @@ -91,6 +95,17 @@ virInterfaceObjSetActive(virInterfaceObjPtr obj, /* virInterfaceObjList manipulation */ +virInterfaceObjListPtr +virInterfaceObjListNew(void) +{ + virInterfaceObjListPtr interfaces; + + if (VIR_ALLOC(interfaces) < 0) + return NULL; + return interfaces; +} + + int virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces, const char *mac, @@ -149,50 +164,50 @@ virInterfaceObjListFree(virInterfaceObjListPtr interfaces) for (i = 0; i < interfaces->count; i++) virInterfaceObjFree(interfaces->objs[i]); - VIR_FREE(interfaces->objs); - interfaces->count = 0; + VIR_FREE(interfaces); } -int -virInterfaceObjListClone(virInterfaceObjListPtr src, - virInterfaceObjListPtr dest) +virInterfaceObjListPtr +virInterfaceObjListClone(virInterfaceObjListPtr interfaces) { - int ret = -1; size_t i; unsigned int cnt; + virInterfaceObjListPtr dest; - if (!src || !dest) - goto cleanup; + if (!interfaces) + return NULL; - virInterfaceObjListFree(dest); /* start with an empty list */ - cnt = src->count; + if (!(dest = virInterfaceObjListNew())) + return NULL; + + cnt = interfaces->count; for (i = 0; i < cnt; i++) { - virInterfaceObjPtr srcobj = src->objs[i]; + virInterfaceObjPtr srcobj = interfaces->objs[i]; virInterfaceDefPtr backup; virInterfaceObjPtr obj; char *xml = virInterfaceDefFormat(srcobj->def); if (!xml) - goto cleanup; + goto error; - if ((backup = virInterfaceDefParseString(xml)) == NULL) { + if (!(backup = virInterfaceDefParseString(xml))) { VIR_FREE(xml); - goto cleanup; + goto error; } VIR_FREE(xml); - if ((obj = virInterfaceObjAssignDef(dest, backup)) == NULL) - goto cleanup; + if (!(obj = virInterfaceObjAssignDef(dest, backup))) + goto error; virInterfaceObjUnlock(obj); /* locked by virInterfaceObjAssignDef */ } - ret = cnt; - cleanup: - if ((ret < 0) && dest) - virInterfaceObjListFree(dest); - return ret; + return dest; + + error: + virInterfaceObjListFree(dest); + return NULL; } diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h index 79b6fc9..19c4947 100644 --- a/src/conf/virinterfaceobj.h +++ b/src/conf/virinterfaceobj.h @@ -27,10 +27,6 @@ typedef virInterfaceObj *virInterfaceObjPtr; typedef struct _virInterfaceObjList virInterfaceObjList; typedef virInterfaceObjList *virInterfaceObjListPtr; -struct _virInterfaceObjList { - size_t count; - virInterfaceObjPtr *objs; -}; virInterfaceDefPtr virInterfaceObjGetDef(virInterfaceObjPtr obj); @@ -42,6 +38,9 @@ void virInterfaceObjSetActive(virInterfaceObjPtr obj, bool active); +virInterfaceObjListPtr +virInterfaceObjListNew(void); + int virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces, const char *mac, @@ -57,9 +56,8 @@ virInterfaceObjFree(virInterfaceObjPtr obj); void virInterfaceObjListFree(virInterfaceObjListPtr vms); -int -virInterfaceObjListClone(virInterfaceObjListPtr src, - virInterfaceObjListPtr dest); +virInterfaceObjListPtr +virInterfaceObjListClone(virInterfaceObjListPtr interfaces); virInterfaceObjPtr virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f9fe871..4ba99ad 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -918,6 +918,7 @@ virInterfaceObjGetNames; virInterfaceObjIsActive; virInterfaceObjListClone; virInterfaceObjListFree; +virInterfaceObjListNew; virInterfaceObjLock; virInterfaceObjNumOfInterfaces; virInterfaceObjRemove; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 412d9f1..2cd55ec 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -97,9 +97,9 @@ struct _testDriver { virMutex lock; virNodeInfo nodeInfo; - virInterfaceObjList ifaces; + virInterfaceObjListPtr ifaces; bool transaction_running; - virInterfaceObjList backupIfaces; + virInterfaceObjListPtr backupIfaces; virStoragePoolObjList pools; virNodeDeviceObjList devs; int numCells; @@ -154,7 +154,7 @@ testDriverFree(testDriverPtr driver) virObjectUnref(driver->domains); virNodeDeviceObjListFree(&driver->devs); virObjectUnref(driver->networks); - virInterfaceObjListFree(&driver->ifaces); + virInterfaceObjListFree(driver->ifaces); virStoragePoolObjListFree(&driver->pools); virObjectUnref(driver->eventState); virMutexUnlock(&driver->lock); @@ -416,6 +416,7 @@ testDriverNew(void) if (!(ret->xmlopt = virDomainXMLOptionNew(NULL, NULL, &ns)) || !(ret->eventState = virObjectEventStateNew()) || + !(ret->ifaces = virInterfaceObjListNew()) || !(ret->domains = virDomainObjListNew()) || !(ret->networks = virNetworkObjListNew())) goto error; @@ -1020,7 +1021,7 @@ testParseInterfaces(testDriverPtr privconn, if (!def) goto error; - if (!(obj = virInterfaceObjAssignDef(&privconn->ifaces, def))) { + if (!(obj = virInterfaceObjAssignDef(privconn->ifaces, def))) { virInterfaceDefFree(def); goto error; } @@ -3630,7 +3631,7 @@ testInterfaceObjFindByName(testDriverPtr privconn, virInterfaceObjPtr obj; testDriverLock(privconn); - obj = virInterfaceObjFindByName(&privconn->ifaces, name); + obj = virInterfaceObjFindByName(privconn->ifaces, name); testDriverUnlock(privconn); if (!obj) @@ -3649,7 +3650,7 @@ testConnectNumOfInterfaces(virConnectPtr conn) int ninterfaces; testDriverLock(privconn); - ninterfaces = virInterfaceObjNumOfInterfaces(&privconn->ifaces, true); + ninterfaces = virInterfaceObjNumOfInterfaces(privconn->ifaces, true); testDriverUnlock(privconn); return ninterfaces; } @@ -3664,7 +3665,7 @@ testConnectListInterfaces(virConnectPtr conn, int nnames; testDriverLock(privconn); - nnames = virInterfaceObjGetNames(&privconn->ifaces, true, names, maxnames); + nnames = virInterfaceObjGetNames(privconn->ifaces, true, names, maxnames); testDriverUnlock(privconn); return nnames; @@ -3678,7 +3679,7 @@ testConnectNumOfDefinedInterfaces(virConnectPtr conn) int ninterfaces; testDriverLock(privconn); - ninterfaces = virInterfaceObjNumOfInterfaces(&privconn->ifaces, false); + ninterfaces = virInterfaceObjNumOfInterfaces(privconn->ifaces, false); testDriverUnlock(privconn); return ninterfaces; } @@ -3693,7 +3694,7 @@ testConnectListDefinedInterfaces(virConnectPtr conn, int nnames; testDriverLock(privconn); - nnames = virInterfaceObjGetNames(&privconn->ifaces, false, names, maxnames); + nnames = virInterfaceObjGetNames(privconn->ifaces, false, names, maxnames); testDriverUnlock(privconn); return nnames; @@ -3731,7 +3732,7 @@ testInterfaceLookupByMACString(virConnectPtr conn, virInterfacePtr ret = NULL; testDriverLock(privconn); - ifacect = virInterfaceObjFindByMACString(&privconn->ifaces, mac, &obj, 1); + ifacect = virInterfaceObjFindByMACString(privconn->ifaces, mac, &obj, 1); testDriverUnlock(privconn); if (ifacect == 0) { @@ -3789,8 +3790,7 @@ testInterfaceChangeBegin(virConnectPtr conn, privconn->transaction_running = true; - if (virInterfaceObjListClone(&privconn->ifaces, - &privconn->backupIfaces) < 0) + if (!(privconn->backupIfaces = virInterfaceObjListClone(privconn->ifaces))) goto cleanup; ret = 0; @@ -3818,7 +3818,7 @@ testInterfaceChangeCommit(virConnectPtr conn, goto cleanup; } - virInterfaceObjListFree(&privconn->backupIfaces); + virInterfaceObjListFree(privconn->backupIfaces); privconn->transaction_running = false; ret = 0; @@ -3848,11 +3848,9 @@ testInterfaceChangeRollback(virConnectPtr conn, goto cleanup; } - virInterfaceObjListFree(&privconn->ifaces); - privconn->ifaces.count = privconn->backupIfaces.count; - privconn->ifaces.objs = privconn->backupIfaces.objs; - privconn->backupIfaces.count = 0; - privconn->backupIfaces.objs = NULL; + virInterfaceObjListFree(privconn->ifaces); + privconn->ifaces = privconn->backupIfaces; + privconn->backupIfaces = NULL; privconn->transaction_running = false; @@ -3903,7 +3901,7 @@ testInterfaceDefineXML(virConnectPtr conn, if (!(def = virInterfaceDefParseString(xmlStr))) goto cleanup; - if ((obj = virInterfaceObjAssignDef(&privconn->ifaces, def)) == NULL) + if ((obj = virInterfaceObjAssignDef(privconn->ifaces, def)) == NULL) goto cleanup; def = NULL; objdef = virInterfaceObjGetDef(obj); @@ -3928,7 +3926,7 @@ testInterfaceUndefine(virInterfacePtr iface) if (!(obj = testInterfaceObjFindByName(privconn, iface->name))) return -1; - virInterfaceObjRemove(&privconn->ifaces, obj); + virInterfaceObjRemove(privconn->ifaces, obj); return 0; } -- 2.9.4

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 9470b4a..8bd8094 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -109,11 +109,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]; @@ -122,18 +122,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); 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]); return ret; } -- 2.9.4

For now it'll just call the virInterfaceObjUnlock, but a future adjustment will do something different. The virInterfaceObjUnlock is now private to virinterfaceobj.c with a short term forward reference. Additionally, make virInterfaceObjLock private since it's only used in virinterfaceobj anyway. For now this will involved creating a forward reference, but this will go away soon too. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virinterfaceobj.c | 17 +++++++++++++++-- src/conf/virinterfaceobj.h | 9 +++------ src/libvirt_private.syms | 3 +-- src/test/test_driver.c | 15 +++++++-------- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index 1e3f25c..8f839b3 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -44,6 +44,9 @@ struct _virInterfaceObjList { virInterfaceObjPtr *objs; }; +static void +virInterfaceObjLock(virInterfaceObjPtr obj); + /* virInterfaceObj manipulation */ static virInterfaceObjPtr @@ -67,14 +70,14 @@ virInterfaceObjNew(void) } -void +static void virInterfaceObjLock(virInterfaceObjPtr obj) { virMutexLock(&obj->lock); } -void +static void virInterfaceObjUnlock(virInterfaceObjPtr obj) { virMutexUnlock(&obj->lock); @@ -82,6 +85,16 @@ virInterfaceObjUnlock(virInterfaceObjPtr obj) void +virInterfaceObjEndAPI(virInterfaceObjPtr *obj) +{ + if (!*obj) + return; + + virInterfaceObjUnlock(*obj); +} + + +void virInterfaceObjFree(virInterfaceObjPtr obj) { if (!obj) diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h index 3934e63..2b9e1b2 100644 --- a/src/conf/virinterfaceobj.h +++ b/src/conf/virinterfaceobj.h @@ -28,6 +28,9 @@ typedef virInterfaceObj *virInterfaceObjPtr; typedef struct _virInterfaceObjList virInterfaceObjList; typedef virInterfaceObjList *virInterfaceObjListPtr; +void +virInterfaceObjEndAPI(virInterfaceObjPtr *obj); + virInterfaceDefPtr virInterfaceObjGetDef(virInterfaceObjPtr obj); @@ -68,12 +71,6 @@ void virInterfaceObjListRemove(virInterfaceObjListPtr interfaces, virInterfaceObjPtr obj); -void -virInterfaceObjLock(virInterfaceObjPtr obj); - -void -virInterfaceObjUnlock(virInterfaceObjPtr obj); - typedef bool (*virInterfaceObjListFilter)(virConnectPtr conn, virInterfaceDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aa6f351..364b32e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -910,6 +910,7 @@ virDomainObjListRename; # conf/virinterfaceobj.h +virInterfaceObjEndAPI; virInterfaceObjGetDef; virInterfaceObjIsActive; virInterfaceObjListAssignDef; @@ -921,9 +922,7 @@ virInterfaceObjListGetNames; virInterfaceObjListNew; virInterfaceObjListNumOfInterfaces; virInterfaceObjListRemove; -virInterfaceObjLock; virInterfaceObjSetActive; -virInterfaceObjUnlock; # conf/virnetworkobj.h diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ac16f4f..fb95319 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1027,7 +1027,7 @@ testParseInterfaces(testDriverPtr privconn, } virInterfaceObjSetActive(obj, true); - virInterfaceObjUnlock(obj); + virInterfaceObjEndAPI(&obj); } ret = 0; @@ -3718,7 +3718,7 @@ testInterfaceLookupByName(virConnectPtr conn, ret = virGetInterface(conn, def->name, def->mac); - virInterfaceObjUnlock(obj); + virInterfaceObjEndAPI(&obj); return ret; } @@ -3769,7 +3769,7 @@ testInterfaceIsActive(virInterfacePtr iface) ret = virInterfaceObjIsActive(obj); - virInterfaceObjUnlock(obj); + virInterfaceObjEndAPI(&obj); return ret; } @@ -3881,7 +3881,7 @@ testInterfaceGetXMLDesc(virInterfacePtr iface, ret = virInterfaceDefFormat(def); - virInterfaceObjUnlock(obj); + virInterfaceObjEndAPI(&obj); return ret; } @@ -3912,8 +3912,7 @@ testInterfaceDefineXML(virConnectPtr conn, cleanup: virInterfaceDefFree(def); - if (obj) - virInterfaceObjUnlock(obj); + virInterfaceObjEndAPI(&obj); testDriverUnlock(privconn); return ret; } @@ -3956,7 +3955,7 @@ testInterfaceCreate(virInterfacePtr iface, ret = 0; cleanup: - virInterfaceObjUnlock(obj); + virInterfaceObjEndAPI(&obj); return ret; } @@ -3983,7 +3982,7 @@ testInterfaceDestroy(virInterfacePtr iface, ret = 0; cleanup: - virInterfaceObjUnlock(obj); + virInterfaceObjEndAPI(&obj); return ret; } -- 2.9.4

On 05/26/2017 08:56 AM, Peter Krempa wrote: Well... It was a way to avoid having more changes in the "next" patch. The reality is this and the next one were one at one time, but I figured I could extract out one and make the following one a bit easier to read. I can combine them into one if that's desired. Tks - John

Rather than pass by value, let's pass by reference since the object ends up "owning" the XML definition, let's make that ownership a bit more real. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virinterfaceobj.c | 12 +++++++----- src/conf/virinterfaceobj.h | 2 +- src/test/test_driver.c | 5 ++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index 51c3c82..f7352d2 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -235,8 +235,10 @@ virInterfaceObjListClone(virInterfaceObjListPtr interfaces) } VIR_FREE(xml); - if (!(obj = virInterfaceObjListAssignDef(dest, backup))) + if (!(obj = virInterfaceObjListAssignDef(dest, &backup))) { + virInterfaceDefFree(backup); goto error; + } virInterfaceObjEndAPI(&obj); } @@ -250,13 +252,13 @@ virInterfaceObjListClone(virInterfaceObjListPtr interfaces) virInterfaceObjPtr virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces, - virInterfaceDefPtr def) + virInterfaceDefPtr *def) { virInterfaceObjPtr obj; - if ((obj = virInterfaceObjListFindByName(interfaces, def->name))) { + if ((obj = virInterfaceObjListFindByName(interfaces, (*def)->name))) { virInterfaceDefFree(obj->def); - obj->def = def; + VIR_STEAL_PTR(obj->def, *def); return obj; } @@ -270,7 +272,7 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces, return NULL; } - obj->def = def; + VIR_STEAL_PTR(obj->def, *def); return virObjectRef(obj); } diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h index 2b9e1b2..0000ee9 100644 --- a/src/conf/virinterfaceobj.h +++ b/src/conf/virinterfaceobj.h @@ -65,7 +65,7 @@ virInterfaceObjListClone(virInterfaceObjListPtr interfaces); virInterfaceObjPtr virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces, - virInterfaceDefPtr def); + virInterfaceDefPtr *def); void virInterfaceObjListRemove(virInterfaceObjListPtr interfaces, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index fb95319..4b4a782 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1021,7 +1021,7 @@ testParseInterfaces(testDriverPtr privconn, if (!def) goto error; - if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces, def))) { + if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces, &def))) { virInterfaceDefFree(def); goto error; } @@ -3903,9 +3903,8 @@ testInterfaceDefineXML(virConnectPtr conn, if (!(def = virInterfaceDefParseString(xmlStr))) goto cleanup; - if ((obj = virInterfaceObjListAssignDef(privconn->ifaces, def)) == NULL) + if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces, &def))) goto cleanup; - def = NULL; objdef = virInterfaceObjGetDef(obj); ret = virGetInterface(conn, objdef->name, objdef->mac); -- 2.9.4