[libvirt] [PATCH 1/2] Remove deallocator parameter from hash functions

Since the deallocator is passed into the constructor of a hash table it is not desirable to pass it into each function again. Remove it from all functions, but provide a virHashSteal to allow a item to be removed from a hash table without deleteing it. * src/util/hash.c, src/util/hash.h: Remove deallocator param from all functions. Add virHashSteal * src/libvirt_private.syms: Add virHashSteal * src/conf/domain_conf.c, src/conf/nwfilter_params.c, src/nwfilter/nwfilter_learnipaddr.c, src/qemu/qemu_command.c, src/xen/xm_internal.c: Update for changed hash API --- src/conf/domain_conf.c | 5 +-- src/conf/nwfilter_params.c | 4 +- src/libvirt_private.syms | 1 + src/nwfilter/nwfilter_learnipaddr.c | 7 +--- src/qemu/qemu_command.c | 2 +- src/util/hash.c | 64 +++++++++++++++++++++++----------- src/util/hash.h | 17 ++++++--- src/xen/xm_internal.c | 24 ++++++------ 8 files changed, 74 insertions(+), 50 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b97c1f0..6270cc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1143,7 +1143,7 @@ void virDomainRemoveInactive(virDomainObjListPtr doms, virDomainObjUnlock(dom); - virHashRemoveEntry(doms->objs, uuidstr, virDomainObjListDeallocator); + virHashRemoveEntry(doms->objs, uuidstr); } @@ -8860,8 +8860,7 @@ virDomainSnapshotObjPtr virDomainSnapshotFindByName(const virDomainSnapshotObjLi void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot) { - virHashRemoveEntry(snapshots->objs, snapshot->def->name, - virDomainSnapshotObjListDeallocator); + virHashRemoveEntry(snapshots->objs, snapshot->def->name); } struct snapshot_has_children { diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index cd94b30..729f455 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -80,7 +80,7 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table, return 1; } } else { - if (virHashUpdateEntry(table->hashTable, name, val, hashDealloc) != 0) { + if (virHashUpdateEntry(table->hashTable, name, val) != 0) { return 1; } } @@ -134,7 +134,7 @@ virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr ht, const char *entry) { int i; - int rc = virHashRemoveEntry(ht->hashTable, entry, hashDealloc); + int rc = virHashRemoveEntry(ht->hashTable, entry); if (rc == 0) { for (i = 0; i < ht->nNames; i++) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 66917ca..55109f2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -413,6 +413,7 @@ virHashRemoveEntry; virHashRemoveSet; virHashSearch; virHashSize; +virHashSteal; # hooks.h diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 8bd7b50..8f6263a 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -207,7 +207,7 @@ virNWFilterUnlockIface(const char *ifname) { ifaceLock->refctr--; if (ifaceLock->refctr == 0) - virHashRemoveEntry(ifaceLockMap, ifname, freeIfaceLock); + virHashRemoveEntry(ifaceLockMap, ifname); } virMutexUnlock(&ifaceMapLock); @@ -301,10 +301,7 @@ virNWFilterDeregisterLearnReq(int ifindex) { virMutexLock(&pendingLearnReqLock); - res = virHashLookup(pendingLearnReq, ifindex_str); - - if (res) - virHashRemoveEntry(pendingLearnReq, ifindex_str, NULL); + res = virHashSteal(pendingLearnReq, ifindex_str); virMutexUnlock(&pendingLearnReqLock); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ddd218f..4bf80d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -842,7 +842,7 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, if (!addr) return -1; - ret = virHashRemoveEntry(addrs->used, addr, qemuDomainPCIAddressSetFreeEntry); + ret = virHashRemoveEntry(addrs->used, addr); VIR_FREE(addr); diff --git a/src/util/hash.c b/src/util/hash.c index 3ab73dd..c3c8b66 100644 --- a/src/util/hash.c +++ b/src/util/hash.c @@ -54,7 +54,7 @@ struct _virHashTable { struct _virHashEntry *table; int size; int nbElems; - virHashDeallocator f; + virHashDataFree dataFree; }; /* @@ -80,14 +80,14 @@ virHashComputeKey(virHashTablePtr table, const char *name) /** * virHashCreate: * @size: the size of the hash table - * @deallocator: function to call on each entry during virHashFree + * @dataFree: function to call on each entry during virHashFree * * Create a new virHashTablePtr. * * Returns the newly created object, or NULL if an error occured. */ virHashTablePtr -virHashCreate(int size, virHashDeallocator deallocator) +virHashCreate(int size, virHashDataFree dataFree) { virHashTablePtr table = NULL; @@ -101,7 +101,7 @@ virHashCreate(int size, virHashDeallocator deallocator) table->size = size; table->nbElems = 0; - table->f = deallocator; + table->dataFree = dataFree; if (VIR_ALLOC_N(table->table, size) < 0) { virReportOOMError(); VIR_FREE(table); @@ -229,8 +229,8 @@ virHashFree(virHashTablePtr table) inside_table = 1; while (iter) { next = iter->next; - if ((table->f != NULL) && (iter->payload != NULL)) - table->f(iter->payload, iter->name); + if ((table->dataFree != NULL) && (iter->payload != NULL)) + table->dataFree(iter->payload, iter->name); VIR_FREE(iter->name); iter->payload = NULL; if (!inside_table) @@ -246,8 +246,8 @@ virHashFree(virHashTablePtr table) } static int -virHashAddOrUpdateEntry(virHashTablePtr table, const char *name, - void *userdata, virHashDeallocator f, +virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, + void *userdata, bool is_update) { unsigned long key, len = 0; @@ -281,8 +281,8 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const char *name, if (found) { if (is_update) { - if (f) - f(insert->payload, insert->name); + if (table->dataFree) + table->dataFree(insert->payload, insert->name); insert->payload = userdata; return (0); } else { @@ -336,7 +336,7 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const char *name, int virHashAddEntry(virHashTablePtr table, const char *name, void *userdata) { - return virHashAddOrUpdateEntry(table, name, userdata, NULL, false); + return virHashAddOrUpdateEntry(table, name, userdata, false); } /** @@ -344,7 +344,6 @@ virHashAddEntry(virHashTablePtr table, const char *name, void *userdata) * @table: the hash table * @name: the name of the userdata * @userdata: a pointer to the userdata - * @f: the deallocator function for replaced item (if any) * * Add the @userdata to the hash @table. This can later be retrieved * by using @name. Existing entry for this tuple @@ -354,9 +353,9 @@ virHashAddEntry(virHashTablePtr table, const char *name, void *userdata) */ int virHashUpdateEntry(virHashTablePtr table, const char *name, - void *userdata, virHashDeallocator f) + void *userdata) { - return virHashAddOrUpdateEntry(table, name, userdata, f, true); + return virHashAddOrUpdateEntry(table, name, userdata, true); } /** @@ -388,6 +387,30 @@ virHashLookup(virHashTablePtr table, const char *name) return (NULL); } + +/** + * virHashSteal: + * @table: the hash table + * @name: the name of the userdata + * + * Find the userdata specified by @name + * and remove it from the hash without freeing it. + * + * Returns the a pointer to the userdata + */ +void *virHashSteal(virHashTablePtr table, const char *name) +{ + void *data = virHashLookup(table, name); + if (data) { + virHashDataFree dataFree = table->dataFree; + table->dataFree = NULL; + virHashRemoveEntry(table, name); + table->dataFree = dataFree; + } + return data; +} + + /** * virHashSize: * @table: the hash table @@ -409,7 +432,6 @@ virHashSize(virHashTablePtr table) * virHashRemoveEntry: * @table: the hash table * @name: the name of the userdata - * @f: the deallocator function for removed item (if any) * * Find the userdata specified by the @name and remove * it from the hash @table. Existing userdata for this tuple will be removed @@ -418,8 +440,7 @@ virHashSize(virHashTablePtr table) * Returns 0 if the removal succeeded and -1 in case of error or not found. */ int -virHashRemoveEntry(virHashTablePtr table, const char *name, - virHashDeallocator f) +virHashRemoveEntry(virHashTablePtr table, const char *name) { unsigned long key; virHashEntryPtr entry; @@ -435,8 +456,8 @@ virHashRemoveEntry(virHashTablePtr table, const char *name, for (entry = &(table->table[key]); entry != NULL; entry = entry->next) { if (STREQ(entry->name, name)) { - if ((f != NULL) && (entry->payload != NULL)) - f(entry->payload, entry->name); + if (table->dataFree && (entry->payload != NULL)) + table->dataFree(entry->payload, entry->name); entry->payload = NULL; VIR_FREE(entry->name); if (prev) { @@ -508,7 +529,7 @@ int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data) { * * Returns number of items removed on success, -1 on failure */ -int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, virHashDeallocator f, const void *data) { +int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void *data) { int i, count = 0; if (table == NULL || iter == NULL) @@ -521,7 +542,8 @@ int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, virHashDealloc while (entry && entry->valid) { if (iter(entry->payload, entry->name, data)) { count++; - f(entry->payload, entry->name); + if (table->dataFree) + table->dataFree(entry->payload, entry->name); VIR_FREE(entry->name); table->nbElems--; if (prev) { diff --git a/src/util/hash.h b/src/util/hash.h index cf08e1a..1147f46 100644 --- a/src/util/hash.h +++ b/src/util/hash.h @@ -23,13 +23,13 @@ typedef virHashTable *virHashTablePtr; */ /** - * virHashDeallocator: + * virHashDataFree: * @payload: the data in the hash * @name: the name associated * * Callback to free data from a hash. */ -typedef void (*virHashDeallocator) (void *payload, const char *name); +typedef void (*virHashDataFree) (void *payload, const char *name); /** * virHashIterator: * @payload: the data in the hash @@ -55,7 +55,7 @@ typedef int (*virHashSearcher) (const void *payload, const char *name, /* * Constructor and destructor. */ -virHashTablePtr virHashCreate(int size, virHashDeallocator f); +virHashTablePtr virHashCreate(int size, virHashDataFree dataFree); void virHashFree(virHashTablePtr table); int virHashSize(virHashTablePtr table); @@ -66,25 +66,30 @@ int virHashAddEntry(virHashTablePtr table, const char *name, void *userdata); int virHashUpdateEntry(virHashTablePtr table, const char *name, - void *userdata, virHashDeallocator f); + void *userdata); /* * Remove an entry from the hash table. */ int virHashRemoveEntry(virHashTablePtr table, - const char *name, virHashDeallocator f); + const char *name); /* * Retrieve the userdata. */ void *virHashLookup(virHashTablePtr table, const char *name); +/* + * Retrieve & remove the userdata. + */ +void *virHashSteal(virHashTablePtr table, const char *name); + /* * Iterators */ int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data); -int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, virHashDeallocator f, const void *data); +int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void *data); void *virHashSearch(virHashTablePtr table, virHashSearcher iter, const void *data); #endif /* ! __VIR_HASH_H__ */ diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 27cc387..62e1068 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -161,7 +161,7 @@ static int xenXMConfigReaper(const void *payload, const char *key ATTRIBUTE_UNUS const char *olddomname = entry->def->name; char *nameowner = (char *)virHashLookup(args->priv->nameConfigMap, olddomname); if (nameowner && STREQ(nameowner, key)) { - virHashRemoveEntry(args->priv->nameConfigMap, olddomname, NULL); + virHashRemoveEntry(args->priv->nameConfigMap, olddomname); } return (1); } @@ -216,8 +216,8 @@ xenXMConfigCacheRemoveFile(virConnectPtr conn, return 0; } - virHashRemoveEntry(priv->nameConfigMap, entry->def->name, NULL); - virHashRemoveEntry(priv->configCache, filename, xenXMConfigFree); + virHashRemoveEntry(priv->nameConfigMap, entry->def->name); + virHashRemoveEntry(priv->configCache, filename); VIR_DEBUG("Removed %s %s", entry->def->name, filename); return 0; } @@ -268,7 +268,7 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename) re-acquire it later - just in case it was renamed */ nameowner = (char *)virHashLookup(priv->nameConfigMap, entry->def->name); if (nameowner && STREQ(nameowner, filename)) { - virHashRemoveEntry(priv->nameConfigMap, entry->def->name, NULL); + virHashRemoveEntry(priv->nameConfigMap, entry->def->name); } /* Clear existing config entry which needs refresh */ @@ -287,7 +287,7 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename) if (!(entry->def = xenXMConfigReadFile(conn, entry->filename))) { VIR_DEBUG("Failed to read %s", entry->filename); if (!newborn) - virHashRemoveEntry(priv->configCache, filename, NULL); + virHashSteal(priv->configCache, filename); VIR_FREE(entry); return -1; } @@ -309,7 +309,7 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename) */ if (!virHashLookup(priv->nameConfigMap, entry->def->name)) { if (virHashAddEntry(priv->nameConfigMap, entry->def->name, entry->filename) < 0) { - virHashRemoveEntry(priv->configCache, filename, NULL); + virHashSteal(priv->configCache, filename); virDomainDefFree(entry->def); VIR_FREE(entry); } @@ -412,7 +412,7 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) { then the config is no longer on disk */ args.now = now; args.priv = priv; - virHashRemoveSet(priv->configCache, xenXMConfigReaper, xenXMConfigFree, &args); + virHashRemoveSet(priv->configCache, xenXMConfigReaper, &args); ret = 0; closedir(dh); @@ -1114,14 +1114,14 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { } /* Remove the name -> filename mapping */ - if (virHashRemoveEntry(priv->nameConfigMap, def->name, NULL) < 0) { + if (virHashRemoveEntry(priv->nameConfigMap, def->name) < 0) { xenXMError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to remove old domain from config map")); goto error; } /* Remove the config record itself */ - if (virHashRemoveEntry(priv->configCache, oldfilename, xenXMConfigFree) < 0) { + if (virHashRemoveEntry(priv->configCache, oldfilename) < 0) { xenXMError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to remove old domain from config map")); goto error; @@ -1164,7 +1164,7 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { } if (virHashAddEntry(priv->nameConfigMap, def->name, entry->filename) < 0) { - virHashRemoveEntry(priv->configCache, filename, NULL); + virHashSteal(priv->configCache, filename); xenXMError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to store config file handle")); goto error; @@ -1213,11 +1213,11 @@ int xenXMDomainUndefine(virDomainPtr domain) { goto cleanup; /* Remove the name -> filename mapping */ - if (virHashRemoveEntry(priv->nameConfigMap, domain->name, NULL) < 0) + if (virHashRemoveEntry(priv->nameConfigMap, domain->name) < 0) goto cleanup; /* Remove the config record itself */ - if (virHashRemoveEntry(priv->configCache, entry->filename, xenXMConfigFree) < 0) + if (virHashRemoveEntry(priv->configCache, entry->filename) < 0) goto cleanup; ret = 0; -- 1.7.4