Upon successful return from virStoragePoolObjListAdd() the virStoragePoolObj is the owner of secret definition. To make this ownership transfer even more visible, lets pass the definition as a double pointer and use g_steal_pointer(). Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/conf/virstorageobj.c | 29 ++++++++++++++++------------- src/conf/virstorageobj.h | 2 +- src/storage/storage_driver.c | 5 ++--- src/test/test_driver.c | 8 +++----- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index f906c5b141..2c29339b6a 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1513,20 +1513,20 @@ virStoragePoolObjSourceFindDuplicate(virStoragePoolObjList *pools, static void virStoragePoolObjAssignDef(virStoragePoolObj *obj, - virStoragePoolDef *def, + virStoragePoolDef **def, unsigned int flags) { if (virStoragePoolObjIsActive(obj) || virStoragePoolObjIsStarting(obj)) { virStoragePoolDefFree(obj->newDef); - obj->newDef = def; + obj->newDef = g_steal_pointer(def); } else { if (!obj->newDef && flags & VIR_STORAGE_POOL_OBJ_LIST_ADD_LIVE) obj->newDef = g_steal_pointer(&obj->def); virStoragePoolDefFree(obj->def); - obj->def = def; + obj->def = g_steal_pointer(def); } } @@ -1548,11 +1548,16 @@ virStoragePoolObjAssignDef(virStoragePoolObj *obj, * If VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE is set in @flags * then this will fail if the pool exists and is active. * + * Upon successful return the virStoragePool object is the owner + * of @def and callers should use virStoragePoolObjGetDef() or + * virStoragePoolObjGetNewDef() if they need to access the + * definition as @def is set to NULL. + * * Returns locked and reffed object pointer or NULL on error */ virStoragePoolObj * virStoragePoolObjListAdd(virStoragePoolObjList *pools, - virStoragePoolDef *def, + virStoragePoolDef **def, unsigned int flags) { virStoragePoolObj *obj = NULL; @@ -1561,10 +1566,10 @@ virStoragePoolObjListAdd(virStoragePoolObjList *pools, virObjectRWLockWrite(pools); - if (virStoragePoolObjSourceFindDuplicate(pools, def) < 0) + if (virStoragePoolObjSourceFindDuplicate(pools, *def) < 0) goto error; - rc = virStoragePoolObjIsDuplicate(pools, def, + rc = virStoragePoolObjIsDuplicate(pools, *def, !!(flags & VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE), &obj); @@ -1579,17 +1584,17 @@ virStoragePoolObjListAdd(virStoragePoolObjList *pools, if (!(obj = virStoragePoolObjNew())) goto error; - virUUIDFormat(def->uuid, uuidstr); + virUUIDFormat((*def)->uuid, uuidstr); if (virHashAddEntry(pools->objs, uuidstr, obj) < 0) goto error; virObjectRef(obj); - if (virHashAddEntry(pools->objsName, def->name, obj) < 0) { + if (virHashAddEntry(pools->objsName, (*def)->name, obj) < 0) { virHashRemoveEntry(pools->objs, uuidstr); goto error; } virObjectRef(obj); - obj->def = def; + obj->def = g_steal_pointer(def); virObjectRWUnlock(pools); return obj; @@ -1620,9 +1625,8 @@ virStoragePoolObjLoad(virStoragePoolObjList *pools, return NULL; } - if (!(obj = virStoragePoolObjListAdd(pools, def, 0))) + if (!(obj = virStoragePoolObjListAdd(pools, &def, 0))) return NULL; - def = NULL; VIR_FREE(obj->configFile); /* for driver reload */ obj->configFile = g_strdup(path); @@ -1673,10 +1677,9 @@ virStoragePoolObjLoadState(virStoragePoolObjList *pools, } /* create the object */ - if (!(obj = virStoragePoolObjListAdd(pools, def, + if (!(obj = virStoragePoolObjListAdd(pools, &def, VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE))) return NULL; - def = NULL; /* XXX: future handling of some additional useful status data, * for now, if a status file for a pool exists, the pool will be marked diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index 65f2ae8175..523bdec244 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -203,7 +203,7 @@ typedef enum { virStoragePoolObj * virStoragePoolObjListAdd(virStoragePoolObjList *pools, - virStoragePoolDef *def, + virStoragePoolDef **def, unsigned int flags); int diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 51daf6a05d..4df2c75a2b 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -746,11 +746,10 @@ storagePoolCreateXML(virConnectPtr conn, if ((backend = virStorageBackendForType(newDef->type)) == NULL) goto cleanup; - if (!(obj = virStoragePoolObjListAdd(driver->pools, newDef, + if (!(obj = virStoragePoolObjListAdd(driver->pools, &newDef, VIR_STORAGE_POOL_OBJ_LIST_ADD_LIVE | VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE))) goto cleanup; - newDef = NULL; def = virStoragePoolObjGetDef(obj); virStoragePoolObjSetStarting(obj, true); @@ -830,7 +829,7 @@ storagePoolDefineXML(virConnectPtr conn, if (virStorageBackendForType(newDef->type) == NULL) goto cleanup; - if (!(obj = virStoragePoolObjListAdd(driver->pools, newDef, 0))) + if (!(obj = virStoragePoolObjListAdd(driver->pools, &newDef, 0))) goto cleanup; newDef = virStoragePoolObjGetNewDef(obj); def = virStoragePoolObjGetDef(obj); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 92c0462170..369edacf29 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1226,7 +1226,7 @@ testParseStorage(testDriver *privconn, if (!def) return -1; - if (!(obj = virStoragePoolObjListAdd(privconn->pools, def, 0))) { + if (!(obj = virStoragePoolObjListAdd(privconn->pools, &def, 0))) { virStoragePoolDefFree(def); return -1; } @@ -6675,10 +6675,9 @@ testStoragePoolCreateXML(virConnectPtr conn, if (!(newDef = virStoragePoolDefParseString(xml, 0))) goto cleanup; - if (!(obj = virStoragePoolObjListAdd(privconn->pools, newDef, + if (!(obj = virStoragePoolObjListAdd(privconn->pools, &newDef, VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE))) goto cleanup; - newDef = NULL; def = virStoragePoolObjGetDef(obj); if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { @@ -6742,9 +6741,8 @@ testStoragePoolDefineXML(virConnectPtr conn, newDef->allocation = defaultPoolAlloc; newDef->available = defaultPoolCap - defaultPoolAlloc; - if (!(obj = virStoragePoolObjListAdd(privconn->pools, newDef, 0))) + if (!(obj = virStoragePoolObjListAdd(privconn->pools, &newDef, 0))) goto cleanup; - newDef = NULL; def = virStoragePoolObjGetDef(obj); event = virStoragePoolEventLifecycleNew(def->name, def->uuid, -- 2.32.0