[libvirt] [PATCH 0/2] Refactor hash tables to store arbitrary keys
This series of two patches does some refactoring of hash tables to allow arbitrary data to be used for hash keys
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
Hi, 2011/2/24 Daniel P. Berrange <berrange@redhat.com>:
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.
Ah, I was planning to send a very similar patch tonight, I had missed the need for virHashSteal though so yours is better. The patch looks good to me. Christophe
Relax the restriction that the hash table key must be a string by allowing an arbitrary hash code generator + comparison func to be provided * util/hash.c, util/hash.h: Allow any pointer as a key * internal.h: Include stdbool.h as standard. * conf/domain_conf.c, conf/domain_conf.c, conf/nwfilter_params.c, nwfilter/nwfilter_gentech_driver.c, nwfilter/nwfilter_gentech_driver.h, nwfilter/nwfilter_learnipaddr.c, qemu/qemu_command.c, qemu/qemu_driver.c, qemu/qemu_process.c, uml/uml_driver.c, xen/xm_internal.c: s/char */void */ in hash callbacks --- src/conf/domain_conf.c | 30 ++++---- src/conf/nwfilter_params.c | 14 ++-- src/lxc/lxc_driver.c | 4 +- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_gentech_driver.h | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 4 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 8 +- src/qemu/qemu_process.c | 4 +- src/uml/uml_driver.c | 4 +- src/util/hash.c | 115 ++++++++++++++++++++++++-------- src/util/hash.h | 69 ++++++++++++++++--- src/xen/xm_internal.c | 8 +- 13 files changed, 185 insertions(+), 81 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6270cc7..e3037fb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -392,7 +392,7 @@ VIR_ENUM_IMPL(virDomainTimerMode, VIR_DOMAIN_TIMER_MODE_LAST, #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE static void -virDomainObjListDeallocator(void *payload, const char *name ATTRIBUTE_UNUSED) +virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { virDomainObjPtr obj = payload; virDomainObjLock(obj); @@ -402,7 +402,7 @@ virDomainObjListDeallocator(void *payload, const char *name ATTRIBUTE_UNUSED) int virDomainObjListInit(virDomainObjListPtr doms) { - doms->objs = virHashCreate(50, virDomainObjListDeallocator); + doms->objs = virHashCreate(50, virDomainObjListDataFree); if (!doms->objs) return -1; return 0; @@ -416,7 +416,7 @@ void virDomainObjListDeinit(virDomainObjListPtr doms) static int virDomainObjListSearchID(const void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, const void *data) { virDomainObjPtr obj = (virDomainObjPtr)payload; @@ -457,7 +457,7 @@ virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms, } static int virDomainObjListSearchName(const void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, const void *data) { virDomainObjPtr obj = (virDomainObjPtr)payload; @@ -8435,7 +8435,7 @@ void virDomainObjUnlock(virDomainObjPtr obj) } -static void virDomainObjListCountActive(void *payload, const char *name ATTRIBUTE_UNUSED, void *data) +static void virDomainObjListCountActive(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { virDomainObjPtr obj = payload; int *count = data; @@ -8445,7 +8445,7 @@ static void virDomainObjListCountActive(void *payload, const char *name ATTRIBUT virDomainObjUnlock(obj); } -static void virDomainObjListCountInactive(void *payload, const char *name ATTRIBUTE_UNUSED, void *data) +static void virDomainObjListCountInactive(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { virDomainObjPtr obj = payload; int *count = data; @@ -8471,7 +8471,7 @@ struct virDomainIDData { int *ids; }; -static void virDomainObjListCopyActiveIDs(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +static void virDomainObjListCopyActiveIDs(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr obj = payload; struct virDomainIDData *data = opaque; @@ -8497,7 +8497,7 @@ struct virDomainNameData { char **const names; }; -static void virDomainObjListCopyInactiveNames(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +static void virDomainObjListCopyInactiveNames(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr obj = payload; struct virDomainNameData *data = opaque; @@ -8753,8 +8753,8 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s /* Snapshot Obj List functions */ static void -virDomainSnapshotObjListDeallocator(void *payload, - const char *name ATTRIBUTE_UNUSED) +virDomainSnapshotObjListDataFree(void *payload, + const void *name ATTRIBUTE_UNUSED) { virDomainSnapshotObjPtr obj = payload; @@ -8763,7 +8763,7 @@ virDomainSnapshotObjListDeallocator(void *payload, int virDomainSnapshotObjListInit(virDomainSnapshotObjListPtr snapshots) { - snapshots->objs = virHashCreate(50, virDomainSnapshotObjListDeallocator); + snapshots->objs = virHashCreate(50, virDomainSnapshotObjListDataFree); if (!snapshots->objs) return -1; return 0; @@ -8783,7 +8783,7 @@ struct virDomainSnapshotNameData { }; static void virDomainSnapshotObjListCopyNames(void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainSnapshotObjPtr obj = payload; @@ -8821,7 +8821,7 @@ cleanup: } static void virDomainSnapshotObjListCount(void *payload ATTRIBUTE_UNUSED, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, void *data) { int *count = data; @@ -8839,7 +8839,7 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots) } static int virDomainSnapshotObjListSearchName(const void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, const void *data) { virDomainSnapshotObjPtr obj = (virDomainSnapshotObjPtr)payload; @@ -8869,7 +8869,7 @@ struct snapshot_has_children { }; static void virDomainSnapshotCountChildren(void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, void *data) { virDomainSnapshotObjPtr obj = payload; diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 729f455..211ff55 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -35,7 +35,7 @@ #define VIR_FROM_THIS VIR_FROM_NWFILTER static void -hashDealloc(void *payload, const char *name ATTRIBUTE_UNUSED) +hashDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { VIR_FREE(payload); } @@ -120,7 +120,7 @@ virNWFilterHashTableCreate(int n) { virReportOOMError(); return NULL; } - ret->hashTable = virHashCreate(n, hashDealloc); + ret->hashTable = virHashCreate(n, hashDataFree); if (!ret->hashTable) { VIR_FREE(ret); return NULL; @@ -157,7 +157,7 @@ struct addToTableStruct { static void -addToTable(void *payload, const char *name, void *data) +addToTable(void *payload, const void *name, void *data) { struct addToTableStruct *atts = (struct addToTableStruct *)data; char *val; @@ -172,10 +172,10 @@ addToTable(void *payload, const char *name, void *data) return; } - if (virNWFilterHashTablePut(atts->target, name, val, 1) != 0) { + if (virNWFilterHashTablePut(atts->target, (const char *)name, val, 1) != 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("Could not put variable '%s' into hashmap"), - name); + (const char *)name); atts->errOccurred = 1; VIR_FREE(val); } @@ -265,13 +265,13 @@ struct formatterParam { static void -_formatParameterAttrs(void *payload, const char *name, void *data) +_formatParameterAttrs(void *payload, const void *name, void *data) { struct formatterParam *fp = (struct formatterParam *)data; virBufferVSprintf(fp->buf, "%s<parameter name='%s' value='%s'/>\n", fp->indent, - name, + (const char *)name, (char *)payload); } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 70e74d5..5b6f784 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1967,7 +1967,7 @@ struct lxcAutostartData { }; static void -lxcAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +lxcAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr vm = payload; const struct lxcAutostartData *data = opaque; @@ -2014,7 +2014,7 @@ lxcAutostartConfigs(lxc_driver_t *driver) { } static void -lxcReconnectVM(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +lxcReconnectVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr vm = payload; lxc_driver_t *driver = opaque; diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index d81aac8..facad13 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -1012,7 +1012,7 @@ virNWFilterTeardownFilter(const virDomainNetDefPtr net) void virNWFilterDomainFWUpdateCB(void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, void *data) { virDomainObjPtr obj = payload; diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index 271bf85..fa86030 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -64,7 +64,7 @@ virNWFilterHashTablePtr virNWFilterCreateVarHashmap(char *macaddr, char *ipaddr); void virNWFilterDomainFWUpdateCB(void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name, void *data); #endif diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 8f6263a..9ee439a 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -189,7 +189,7 @@ virNWFilterLockIface(const char *ifname) { static void -freeIfaceLock(void *payload, const char *name ATTRIBUTE_UNUSED) { +freeIfaceLock(void *payload, const void *name ATTRIBUTE_UNUSED) { VIR_FREE(payload); } @@ -287,7 +287,7 @@ virNWFilterLookupLearnReq(int ifindex) { static void -freeLearnReqEntry(void *payload, const char *name ATTRIBUTE_UNUSED) { +freeLearnReqEntry(void *payload, const void *name ATTRIBUTE_UNUSED) { virNWFilterIPAddrLearnReqFree(payload); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4bf80d0..fa60abe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -748,7 +748,7 @@ cleanup: static void qemuDomainPCIAddressSetFreeEntry(void *payload, - const char *name ATTRIBUTE_UNUSED) + const void *name ATTRIBUTE_UNUSED) { VIR_FREE(payload); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 102f780..0fe0bc8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -132,7 +132,7 @@ struct qemuAutostartData { }; static void -qemuAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr vm = payload; struct qemuAutostartData *data = opaque; @@ -275,7 +275,7 @@ err_exit: } static void qemuDomainSnapshotLoad(void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, void *data) { virDomainObjPtr vm = (virDomainObjPtr)payload; @@ -6508,7 +6508,7 @@ struct snap_remove { }; static void qemuDomainSnapshotDiscardChildren(void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, void *data) { virDomainSnapshotObjPtr snap = payload; @@ -6539,7 +6539,7 @@ struct snap_reparent { static void qemuDomainSnapshotReparentChildren(void *payload, - const char *name ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, void *data) { virDomainSnapshotObjPtr snap = payload; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7879165..c419c75 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -949,7 +949,7 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm, return 0; } -static void qemuProcessFreePtyPath(void *payload, const char *name ATTRIBUTE_UNUSED) +static void qemuProcessFreePtyPath(void *payload, const void *name ATTRIBUTE_UNUSED) { VIR_FREE(payload); } @@ -1783,7 +1783,7 @@ struct qemuProcessReconnectData { * and re-reserve the security labels in use */ static void -qemuProcessReconnect(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr obj = payload; struct qemuProcessReconnectData *data = opaque; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 2af8002..7cacadb 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -149,7 +149,7 @@ struct umlAutostartData { }; static void -umlAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +umlAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr vm = payload; const struct umlAutostartData *data = opaque; @@ -508,7 +508,7 @@ umlActive(void) { } static void -umlShutdownOneVM(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaque) +umlShutdownOneVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { virDomainObjPtr dom = payload; struct uml_driver *driver = opaque; diff --git a/src/util/hash.c b/src/util/hash.c index c3c8b66..92ee234 100644 --- a/src/util/hash.c +++ b/src/util/hash.c @@ -42,7 +42,7 @@ typedef struct _virHashEntry virHashEntry; typedef virHashEntry *virHashEntryPtr; struct _virHashEntry { struct _virHashEntry *next; - char *name; + void *name; void *payload; int valid; }; @@ -55,39 +55,70 @@ struct _virHashTable { int size; int nbElems; virHashDataFree dataFree; + virHashKeyCode keyCode; + virHashKeyEqual keyEqual; + virHashKeyCopy keyCopy; + virHashKeyFree keyFree; }; -/* - * virHashComputeKey: - * Calculate the hash key - */ -static unsigned long -virHashComputeKey(virHashTablePtr table, const char *name) +static unsigned long virHashStrCode(const void *name) { + const char *str = name; unsigned long value = 0L; char ch; - if (name != NULL) { - value += 30 * (*name); - while ((ch = *name++) != 0) { + if (str != NULL) { + value += 30 * (*str); + while ((ch = *str++) != 0) { value = value ^ ((value << 5) + (value >> 3) + (unsigned long) ch); } } + return value; +} + +static bool virHashStrEqual(const void *namea, const void *nameb) +{ + return STREQ(namea, nameb); +} + +static void *virHashStrCopy(const void *name) +{ + return strdup(name); +} + +static void virHashStrFree(void *name) +{ + VIR_FREE(name); +} + + +static unsigned long +virHashComputeKey(virHashTablePtr table, const void *name) +{ + unsigned long value = table->keyCode(name); return (value % table->size); } /** - * virHashCreate: + * virHashCreateFull: * @size: the size of the hash table - * @dataFree: function to call on each entry during virHashFree + * @dataFree: callback to free data + * @keyCode: callback to compute hash code + * @keyEqual: callback to compare hash keys + * @keyCopy: callback to copy hash keys + * @keyFree: callback to free keys * * Create a new virHashTablePtr. * * Returns the newly created object, or NULL if an error occured. */ -virHashTablePtr -virHashCreate(int size, virHashDataFree dataFree) +virHashTablePtr virHashCreateFull(int size, + virHashDataFree dataFree, + virHashKeyCode keyCode, + virHashKeyEqual keyEqual, + virHashKeyCopy keyCopy, + virHashKeyFree keyFree) { virHashTablePtr table = NULL; @@ -102,6 +133,11 @@ virHashCreate(int size, virHashDataFree dataFree) table->size = size; table->nbElems = 0; table->dataFree = dataFree; + table->keyCode = keyCode; + table->keyEqual = keyEqual; + table->keyCopy = keyCopy; + table->keyFree = keyFree; + if (VIR_ALLOC_N(table->table, size) < 0) { virReportOOMError(); VIR_FREE(table); @@ -111,6 +147,26 @@ virHashCreate(int size, virHashDataFree dataFree) return table; } + +/** + * virHashCreate: + * @size: the size of the hash table + * @dataFree: callback to free data + * + * Create a new virHashTablePtr. + * + * Returns the newly created object, or NULL if an error occured. + */ +virHashTablePtr virHashCreate(int size, virHashDataFree dataFree) +{ + return virHashCreateFull(size, + dataFree, + virHashStrCode, + virHashStrEqual, + virHashStrCopy, + virHashStrFree); +} + /** * virHashGrow: * @table: the hash table @@ -231,7 +287,8 @@ virHashFree(virHashTablePtr table) next = iter->next; if ((table->dataFree != NULL) && (iter->payload != NULL)) table->dataFree(iter->payload, iter->name); - VIR_FREE(iter->name); + if (table->keyFree) + table->keyFree(iter->name); iter->payload = NULL; if (!inside_table) VIR_FREE(iter); @@ -269,13 +326,13 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, } else { for (insert = &(table->table[key]); insert->next != NULL; insert = insert->next) { - if (STREQ(insert->name, name)) { + if (table->keyEqual(insert->name, name)) { found = true; break; } len++; } - if (STREQ(insert->name, name)) + if (table->keyEqual(insert->name, name)) found = true; } @@ -299,7 +356,7 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, } } - new_name = strdup(name); + new_name = table->keyCopy(name); if (new_name == NULL) { virReportOOMError(); if (insert != NULL) @@ -334,7 +391,7 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, * Returns 0 the addition succeeded and -1 in case of error. */ int -virHashAddEntry(virHashTablePtr table, const char *name, void *userdata) +virHashAddEntry(virHashTablePtr table, const void *name, void *userdata) { return virHashAddOrUpdateEntry(table, name, userdata, false); } @@ -352,7 +409,7 @@ virHashAddEntry(virHashTablePtr table, const char *name, void *userdata) * Returns 0 the addition succeeded and -1 in case of error. */ int -virHashUpdateEntry(virHashTablePtr table, const char *name, +virHashUpdateEntry(virHashTablePtr table, const void *name, void *userdata) { return virHashAddOrUpdateEntry(table, name, userdata, true); @@ -363,12 +420,12 @@ virHashUpdateEntry(virHashTablePtr table, const char *name, * @table: the hash table * @name: the name of the userdata * - * Find the userdata specified by the (@name, @name2, @name3) tuple. + * Find the userdata specified by @name * * Returns the a pointer to the userdata */ void * -virHashLookup(virHashTablePtr table, const char *name) +virHashLookup(virHashTablePtr table, const void *name) { unsigned long key; virHashEntryPtr entry; @@ -381,7 +438,7 @@ virHashLookup(virHashTablePtr table, const char *name) if (table->table[key].valid == 0) return (NULL); for (entry = &(table->table[key]); entry != NULL; entry = entry->next) { - if (STREQ(entry->name, name)) + if (table->keyEqual(entry->name, name)) return (entry->payload); } return (NULL); @@ -398,7 +455,7 @@ virHashLookup(virHashTablePtr table, const char *name) * * Returns the a pointer to the userdata */ -void *virHashSteal(virHashTablePtr table, const char *name) +void *virHashSteal(virHashTablePtr table, const void *name) { void *data = virHashLookup(table, name); if (data) { @@ -440,7 +497,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) +virHashRemoveEntry(virHashTablePtr table, const void *name) { unsigned long key; virHashEntryPtr entry; @@ -455,11 +512,12 @@ virHashRemoveEntry(virHashTablePtr table, const char *name) } else { for (entry = &(table->table[key]); entry != NULL; entry = entry->next) { - if (STREQ(entry->name, name)) { + if (table->keyEqual(entry->name, name)) { if (table->dataFree && (entry->payload != NULL)) table->dataFree(entry->payload, entry->name); entry->payload = NULL; - VIR_FREE(entry->name); + if (table->keyFree) + table->keyFree(entry->name); if (prev) { prev->next = entry->next; VIR_FREE(entry); @@ -544,7 +602,8 @@ int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void *da count++; if (table->dataFree) table->dataFree(entry->payload, entry->name); - VIR_FREE(entry->name); + if (table->keyFree) + table->keyFree(entry->name); table->nbElems--; if (prev) { prev->next = entry->next; diff --git a/src/util/hash.h b/src/util/hash.h index 1147f46..44ac1fb 100644 --- a/src/util/hash.h +++ b/src/util/hash.h @@ -29,33 +29,78 @@ typedef virHashTable *virHashTablePtr; * * Callback to free data from a hash. */ -typedef void (*virHashDataFree) (void *payload, const char *name); +typedef void (*virHashDataFree) (void *payload, const void *name); /** * virHashIterator: * @payload: the data in the hash - * @name: the name associated + * @name: the hash key * @data: user supplied data blob * * Callback to process a hash entry during iteration */ -typedef void (*virHashIterator) (void *payload, const char *name, void *data); +typedef void (*virHashIterator) (void *payload, const void *name, void *data); /** - * virHashSearcher + * virHashSearcher: * @payload: the data in the hash - * @name: the name associated + * @name: the hash key * @data: user supplied data blob * * Callback to identify hash entry desired * Returns 1 if the hash entry is desired, 0 to move * to next entry */ -typedef int (*virHashSearcher) (const void *payload, const char *name, +typedef int (*virHashSearcher) (const void *payload, const void *name, const void *data); +/** + * virHashKeyCode: + * @name: the hash key + * + * Compute the hash code corresponding to the key @name + * + * Returns the hash code + */ +typedef unsigned long (*virHashKeyCode)(const void *name); +/** + * virHashKeyEqual: + * @namea: the first hash key + * @nameb: the second hash key + * + * Compare two hash keys for equality + * + * Returns true if the keys are equal, false otherwise + */ +typedef bool (*virHashKeyEqual)(const void *namea, const void *nameb); +/** + * virHashKeyCopy: + * @name: the hash key + * + * Create a copy of the hash key, duplicating + * memory allocation where applicable + * + * Returns a newly allocated copy of @name + */ +typedef void *(*virHashKeyCopy)(const void *name); +/** + * virHashKeyFree: + * @name: the hash key + * + * Free any memory associated with the hash + * key @name + */ +typedef void (*virHashKeyFree)(void *name); + /* * Constructor and destructor. */ -virHashTablePtr virHashCreate(int size, virHashDataFree dataFree); +virHashTablePtr virHashCreate(int size, + virHashDataFree dataFree); +virHashTablePtr virHashCreateFull(int size, + virHashDataFree dataFree, + virHashKeyCode keyCode, + virHashKeyEqual keyEqual, + virHashKeyCopy keyCopy, + virHashKeyFree keyFree); void virHashFree(virHashTablePtr table); int virHashSize(virHashTablePtr table); @@ -63,26 +108,26 @@ int virHashSize(virHashTablePtr table); * Add a new entry to the hash table. */ int virHashAddEntry(virHashTablePtr table, - const char *name, void *userdata); + const void *name, void *userdata); int virHashUpdateEntry(virHashTablePtr table, - const char *name, + const void *name, void *userdata); /* * Remove an entry from the hash table. */ int virHashRemoveEntry(virHashTablePtr table, - const char *name); + const void *name); /* * Retrieve the userdata. */ -void *virHashLookup(virHashTablePtr table, const char *name); +void *virHashLookup(virHashTablePtr table, const void *name); /* * Retrieve & remove the userdata. */ -void *virHashSteal(virHashTablePtr table, const char *name); +void *virHashSteal(virHashTablePtr table, const void *name); /* diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 62e1068..7f73588 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -139,7 +139,7 @@ static int xenInotifyActive(virConnectPtr conn) /* Release memory associated with a cached config object */ -static void xenXMConfigFree(void *payload, const char *key ATTRIBUTE_UNUSED) { +static void xenXMConfigFree(void *payload, const void *key ATTRIBUTE_UNUSED) { xenXMConfCachePtr entry = (xenXMConfCachePtr)payload; virDomainDefFree(entry->def); VIR_FREE(entry); @@ -151,7 +151,7 @@ struct xenXMConfigReaperData { }; /* Remove any configs which were not refreshed recently */ -static int xenXMConfigReaper(const void *payload, const char *key ATTRIBUTE_UNUSED, const void *data) { +static int xenXMConfigReaper(const void *payload, const void *key ATTRIBUTE_UNUSED, const void *data) { const struct xenXMConfigReaperData *args = data; xenXMConfCachePtr entry = (xenXMConfCachePtr)payload; @@ -938,7 +938,7 @@ cleanup: /* * Hash table iterator to search for a domain based on UUID */ -static int xenXMDomainSearchForUUID(const void *payload, const char *name ATTRIBUTE_UNUSED, const void *data) { +static int xenXMDomainSearchForUUID(const void *payload, const void *name ATTRIBUTE_UNUSED, const void *data) { const unsigned char *wantuuid = (const unsigned char *)data; const xenXMConfCachePtr entry = (const xenXMConfCachePtr)payload; @@ -1235,7 +1235,7 @@ struct xenXMListIteratorContext { char ** names; }; -static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const char *name, void *data) { +static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) { struct xenXMListIteratorContext *ctx = data; virDomainPtr dom = NULL; -- 1.7.4
Hi, On Thu, Feb 24, 2011 at 01:24:32PM +0000, Daniel P. Berrange wrote:
diff --git a/src/util/hash.c b/src/util/hash.c index c3c8b66..92ee234 100644 --- a/src/util/hash.c +++ b/src/util/hash.c @@ -55,39 +55,70 @@ struct _virHashTable { int size; int nbElems; virHashDataFree dataFree; + virHashKeyCode keyCode; + virHashKeyEqual keyEqual; + virHashKeyCopy keyCopy; + virHashKeyFree keyFree;
The new names are much clearer, thanks :) Rest of the patch looks good to me. Christophe
participants (2)
-
Christophe Fergeau -
Daniel P. Berrange