[PATCH 00/15] util: hash: Use glib's GHashTable - preparation (part 1)

Glib provides a hash table implementation called GHashTable. In this part of the series we'll refactor two instances of code which use non-string keys for hashtable to use GHashTable directly which simplifies the code (glib provides int hashing functions). Since GHashTable is not a direct replacement for virHashTable without code modification (glib's functions don't accept NULL hash table, ours do) the next step will be to use virHashTable as a shim to provide NULL compatibility and adapt to our slightly different style of iterators. For this step we modify the variable type for the key to be 'char *' as there's no other option left and remove various internals which won't be compatible with GHashTable. Second part (WIP, [1]) will then rewrite virHashTable internals to use GHashTable, which will be used as an intermediate step before removal which requires audit of all callers. [1]: https://gitlab.com/pipo.sk/libvirt/-/commits/glib-hash-part2 - needs auditing of all callers of virHashForeach to ensure that they don't modify the hash table itself from the callback Peter Krempa (15): virCgroupKillRecursive: Return -1 on failure condition util: virhash: Remove virHashTableSize util: cgroup: Use GHashTable instead of virHashTable virCgroupKillRecursive: Refactor cleanup conf: domain_addr: Refactor hash usage in zpci reservation code virHashAtomicNew: Remove 'size' argument conf: nwfilter: Replace 'virNWFilterHashTableCreate' with 'virHashNew' tests: hash: Prepare for replacement of virHashCreate qemuDomainObjPrivateAlloc: Use virHashNew instead of virHashCreate Replace all instances of 'virHashCreate' with 'virHashNew' util: hash: Remove virHashValueFree util: hash: Remove virHashCreateFull util: hash: Change type of hash table name/key to 'char' util: virhash: Remove key handling callbacks virHashRemoveAll: Don't return number of removed items src/conf/domain_addr.c | 125 +++-------- src/conf/domain_addr.h | 4 +- src/conf/domain_conf.c | 4 +- src/conf/domain_nwfilter.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 2 +- src/conf/nwfilter_params.c | 14 +- src/conf/nwfilter_params.h | 2 +- src/conf/virchrdev.c | 4 +- src/conf/virdomainmomentobjlist.c | 8 +- src/conf/virdomainobjlist.c | 16 +- src/conf/virinterfaceobj.c | 12 +- src/conf/virnetworkobj.c | 21 +- src/conf/virnodedeviceobj.c | 20 +- src/conf/virnwfilterbindingdef.c | 2 +- src/conf/virnwfilterbindingobjlist.c | 6 +- src/conf/virsecretobj.c | 10 +- src/conf/virstorageobj.c | 32 +-- src/hyperv/hyperv_wmi.c | 2 +- src/hypervisor/virclosecallbacks.c | 4 +- src/libvirt_private.syms | 5 +- src/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 10 +- src/nwfilter/nwfilter_dhcpsnoop.c | 12 +- src/nwfilter/nwfilter_ebiptables_driver.c | 4 +- src/nwfilter/nwfilter_gentech_driver.c | 14 +- src/nwfilter/nwfilter_learnipaddr.c | 4 +- src/qemu/qemu_block.c | 6 +- src/qemu/qemu_blockjob.c | 2 +- src/qemu/qemu_capabilities.c | 6 +- src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_domain.c | 8 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_interop_config.c | 2 +- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_monitor.c | 10 +- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_qapi.c | 2 +- src/qemu/qemu_snapshot.c | 4 +- src/rpc/virnetdaemon.c | 16 +- src/security/security_selinux.c | 4 +- src/test/test_driver.c | 6 +- src/util/vircgroup.c | 76 ++----- src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 2 +- src/util/vircgroupv2.c | 2 +- src/util/virfilecache.c | 4 +- src/util/virhash.c | 201 +++--------------- src/util/virhash.h | 94 ++------ src/util/viriptables.c | 4 +- src/util/virlockspace.c | 8 +- src/util/virmacmap.c | 6 +- src/util/virstoragefile.c | 4 +- src/util/virsystemd.c | 2 +- tests/nwfilterxml2firewalltest.c | 6 +- tests/qemumonitorjsontest.c | 12 +- tests/qemusecuritymock.c | 12 +- .../blockjob-blockdev-in.xml | 110 +++++----- tests/testutilsqemu.c | 2 +- tests/virhashtest.c | 52 ++--- 63 files changed, 340 insertions(+), 683 deletions(-) -- 2.26.2

virCgroupKillRecursive sneakily initializes 'ret' to 0 rather than the usual -1. 401030499bf moved an error condition but didn't actually modify 'ret' return the proper error code. Fixes: 401030499bf Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index d408e3366f..5f4cb01bc0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2591,6 +2591,7 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) if (!backends || !backendAvailable) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no cgroup backend available")); + ret = -1; goto cleanup; } -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:52AM +0200, Peter Krempa wrote:
virCgroupKillRecursive sneakily initializes 'ret' to 0 rather than the usual -1. 401030499bf moved an error condition but didn't actually modify 'ret' return the proper error code.
Fixes: 401030499bf Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

It's used only in one place in tests which isn't even automatically evaluated. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virhash.c | 17 ----------------- src/util/virhash.h | 1 - tests/virhashtest.c | 6 ------ 4 files changed, 25 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b2d786409b..79c8403208 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2213,7 +2213,6 @@ virHashRemoveSet; virHashSearch; virHashSize; virHashSteal; -virHashTableSize; virHashUpdateEntry; virHashValueFree; diff --git a/src/util/virhash.c b/src/util/virhash.c index 4b503a0313..7dd2d9f81d 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -549,23 +549,6 @@ virHashSize(const virHashTable *table) return table->nbElems; } -/** - * virHashTableSize: - * @table: the hash table - * - * Query the size of the hash @table, i.e., number of buckets in the table. - * - * Returns the number of keys in the hash table or - * -1 in case of error - */ -ssize_t -virHashTableSize(const virHashTable *table) -{ - if (table == NULL) - return -1; - return table->size; -} - /** * virHashRemoveEntry: diff --git a/src/util/virhash.h b/src/util/virhash.h index cb59fb639b..37853aba36 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -125,7 +125,6 @@ virHashTablePtr virHashCreateFull(ssize_t size, virHashKeyFree keyFree); void virHashFree(virHashTablePtr table); ssize_t virHashSize(const virHashTable *table); -ssize_t virHashTableSize(const virHashTable *table); /* * Add a new entry to the hash table. diff --git a/tests/virhashtest.c b/tests/virhashtest.c index af30791241..7b1b8dd38c 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -27,16 +27,10 @@ testHashInit(int size) * collision list in the same order as in the uuids array */ for (i = G_N_ELEMENTS(uuids) - 1; i >= 0; i--) { - ssize_t oldsize = virHashTableSize(hash); if (virHashAddEntry(hash, uuids[i], (void *) uuids[i]) < 0) { virHashFree(hash); return NULL; } - - if (virHashTableSize(hash) != oldsize) { - VIR_TEST_DEBUG("hash grown from %zu to %zu", - (size_t)oldsize, (size_t)virHashTableSize(hash)); - } } for (i = 0; i < G_N_ELEMENTS(uuids); i++) { -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:53AM +0200, Peter Krempa wrote:
It's used only in one place in tests which isn't even automatically evaluated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virhash.c | 17 ----------------- src/util/virhash.h | 1 - tests/virhashtest.c | 6 ------ 4 files changed, 25 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Rewrite using GHashTable which already has interfaces for using a number as hash key. Glib's implementation doesn't copy the key by default, so we need to allocate it, but overal the interface is more suited for this case. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 61 ++++++++----------------------------- src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 2 +- src/util/vircgroupv2.c | 2 +- 5 files changed, 17 insertions(+), 53 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5f4cb01bc0..b74ec1a8fa 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -42,7 +42,6 @@ #include "virlog.h" #include "virfile.h" #include "virhash.h" -#include "virhashcode.h" #include "virstring.h" #include "virsystemd.h" #include "virtypedparam.h" @@ -2382,7 +2381,7 @@ virCgroupRemove(virCgroupPtr group) static int virCgroupKillInternal(virCgroupPtr group, int signum, - virHashTablePtr pids, + GHashTable *pids, int controller, const char *taskFile) { @@ -2415,8 +2414,9 @@ virCgroupKillInternal(virCgroupPtr group, goto cleanup; } else { while (!feof(fp)) { - long pid_value; - if (fscanf(fp, "%ld", &pid_value) != 1) { + g_autofree long long *pid_value = g_new0(long long, 1); + + if (fscanf(fp, "%lld", pid_value) != 1) { if (feof(fp)) break; virReportSystemError(errno, @@ -2424,16 +2424,17 @@ virCgroupKillInternal(virCgroupPtr group, keypath); goto cleanup; } - if (virHashLookup(pids, (void*)pid_value)) + + if (g_hash_table_lookup(pids, pid_value)) continue; - VIR_DEBUG("pid=%ld", pid_value); + VIR_DEBUG("pid=%lld", *pid_value); /* Cgroups is a Linux concept, so this cast is safe. */ - if (kill((pid_t)pid_value, signum) < 0) { + if (kill((pid_t)*pid_value, signum) < 0) { if (errno != ESRCH) { virReportSystemError(errno, - _("Failed to kill process %ld"), - pid_value); + _("Failed to kill process %lld"), + *pid_value); goto cleanup; } /* Leave RC == 0 since we didn't kill one */ @@ -2442,7 +2443,7 @@ virCgroupKillInternal(virCgroupPtr group, done = false; } - ignore_value(virHashAddEntry(pids, (void*)pid_value, (void*)1)); + g_hash_table_add(pids, g_steal_pointer(&pid_value)); } VIR_FORCE_FCLOSE(fp); } @@ -2458,39 +2459,10 @@ virCgroupKillInternal(virCgroupPtr group, } -static uint32_t -virCgroupPidCode(const void *name, uint32_t seed) -{ - long pid_value = (long)(intptr_t)name; - return virHashCodeGen(&pid_value, sizeof(pid_value), seed); -} - - -static bool -virCgroupPidEqual(const void *namea, const void *nameb) -{ - return namea == nameb; -} - - -static void * -virCgroupPidCopy(const void *name) -{ - return (void*)name; -} - - -static char * -virCgroupPidPrintHuman(const void *name) -{ - return g_strdup_printf("%ld", (const long)name); -} - - int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, - virHashTablePtr pids, + GHashTable *pids, int controller, const char *taskFile, bool dormdir) @@ -2565,13 +2537,7 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) size_t i; bool backendAvailable = false; virCgroupBackendPtr *backends = virCgroupBackendGetAll(); - virHashTablePtr pids = virHashCreateFull(100, - NULL, - virCgroupPidCode, - virCgroupPidEqual, - virCgroupPidCopy, - virCgroupPidPrintHuman, - NULL); + g_autoptr(GHashTable) pids = g_hash_table_new_full(g_int64_hash, g_int64_equal, g_free, NULL); VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); @@ -2596,7 +2562,6 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) } cleanup: - virHashFree(pids); return ret; } diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index bcbe435d78..f677157a91 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -23,7 +23,6 @@ #include "internal.h" #include "vircgroup.h" -#include "virhash.h" #define CGROUP_MAX_VAL 512 @@ -136,7 +135,7 @@ typedef int typedef int (*virCgroupKillRecursiveCB)(virCgroupPtr group, int signum, - virHashTablePtr pids); + GHashTable *pids); typedef int (*virCgroupBindMountCB)(virCgroupPtr group, diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index f2a80aeb82..f85a36efdb 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -136,7 +136,7 @@ int virCgroupRemoveRecursively(char *grppath); int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, - virHashTablePtr pids, + GHashTable *pids, int controller, const char *taskFile, bool dormdir); diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index a42b7750f9..91c1c4d4b1 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -752,7 +752,7 @@ virCgroupV1HasEmptyTasks(virCgroupPtr cgroup, static int virCgroupV1KillRecursive(virCgroupPtr group, int signum, - virHashTablePtr pids) + GHashTable *pids) { int controller = virCgroupV1GetAnyController(group); diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index ddbe3d6663..285b7675d9 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -533,7 +533,7 @@ virCgroupV2HasEmptyTasks(virCgroupPtr cgroup, static int virCgroupV2KillRecursive(virCgroupPtr group, int signum, - virHashTablePtr pids) + GHashTable *pids) { int controller = virCgroupV2GetAnyController(group); -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
Rewrite using GHashTable which already has interfaces for using a number as hash key. Glib's implementation doesn't copy the key by default, so we need to allocate it, but overal the interface is more suited for this case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 61 ++++++++----------------------------- src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 2 +- src/util/vircgroupv2.c | 2 +- 5 files changed, 17 insertions(+), 53 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5f4cb01bc0..b74ec1a8fa 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -42,7 +42,6 @@ #include "virlog.h" #include "virfile.h" #include "virhash.h" -#include "virhashcode.h" #include "virstring.h" #include "virsystemd.h" #include "virtypedparam.h" @@ -2382,7 +2381,7 @@ virCgroupRemove(virCgroupPtr group) static int virCgroupKillInternal(virCgroupPtr group, int signum, - virHashTablePtr pids, + GHashTable *pids, int controller, const char *taskFile) { @@ -2415,8 +2414,9 @@ virCgroupKillInternal(virCgroupPtr group, goto cleanup; } else { while (!feof(fp)) { - long pid_value; - if (fscanf(fp, "%ld", &pid_value) != 1) { + g_autofree long long *pid_value = g_new0(long long, 1);
I would rather use gint64 here as the exact type of gint64 changes with the hardware. For example on my AMD x86_84 it is 'signed long'. We should do this every time we pass pointers to GLib APIs because for example bool and gboolean are different and when I used bool type in GLib dbus APIs it randomly crashed.
+ if (fscanf(fp, "%lld", pid_value) != 1) { if (feof(fp)) break; virReportSystemError(errno, @@ -2424,16 +2424,17 @@ virCgroupKillInternal(virCgroupPtr group, keypath); goto cleanup; } - if (virHashLookup(pids, (void*)pid_value)) + + if (g_hash_table_lookup(pids, pid_value)) continue;
- VIR_DEBUG("pid=%ld", pid_value); + VIR_DEBUG("pid=%lld", *pid_value);
Using gint64 would require to use G_GINT64_FORMAT here. Otherwise looks good. Pavel

On Thu, Oct 22, 2020 at 03:11:59PM +0200, Pavel Hrdina wrote:
On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
Rewrite using GHashTable which already has interfaces for using a number as hash key. Glib's implementation doesn't copy the key by default, so we need to allocate it, but overal the interface is more suited for this case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 61 ++++++++----------------------------- src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 2 +- src/util/vircgroupv2.c | 2 +- 5 files changed, 17 insertions(+), 53 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5f4cb01bc0..b74ec1a8fa 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -42,7 +42,6 @@ #include "virlog.h" #include "virfile.h" #include "virhash.h" -#include "virhashcode.h" #include "virstring.h" #include "virsystemd.h" #include "virtypedparam.h" @@ -2382,7 +2381,7 @@ virCgroupRemove(virCgroupPtr group) static int virCgroupKillInternal(virCgroupPtr group, int signum, - virHashTablePtr pids, + GHashTable *pids, int controller, const char *taskFile) { @@ -2415,8 +2414,9 @@ virCgroupKillInternal(virCgroupPtr group, goto cleanup; } else { while (!feof(fp)) { - long pid_value; - if (fscanf(fp, "%ld", &pid_value) != 1) { + g_autofree long long *pid_value = g_new0(long long, 1);
I would rather use gint64 here as the exact type of gint64 changes with the hardware. For example on my AMD x86_84 it is 'signed long'.
If you use gint64, then you need to start using PRIu64 macro to deal with the fact that the data type changes for printf formatting. Using long long is simpler as you can unconditionally use %ll which is a good thing IMHO.
We should do this every time we pass pointers to GLib APIs because for example bool and gboolean are different and when I used bool type in GLib dbus APIs it randomly crashed.
bool vs gboolean is a special case because of the different types. For all the other g* basic types, we should not use them. GLib has a ticket open considering deprecating them, because they re-invent stdint.h
- VIR_DEBUG("pid=%ld", pid_value); + VIR_DEBUG("pid=%lld", *pid_value);
Using gint64 would require to use G_GINT64_FORMAT here.
That is exactly why we should not use guint64 - it makes the debug code more verbose for no benefit. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, Oct 22, 2020 at 14:17:01 +0100, Daniel Berrange wrote:
On Thu, Oct 22, 2020 at 03:11:59PM +0200, Pavel Hrdina wrote:
On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
Rewrite using GHashTable which already has interfaces for using a number as hash key. Glib's implementation doesn't copy the key by default, so we need to allocate it, but overal the interface is more suited for this case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 61 ++++++++----------------------------- src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 2 +- src/util/vircgroupv2.c | 2 +- 5 files changed, 17 insertions(+), 53 deletions(-)
[...]
@@ -2415,8 +2414,9 @@ virCgroupKillInternal(virCgroupPtr group, goto cleanup; } else { while (!feof(fp)) { - long pid_value; - if (fscanf(fp, "%ld", &pid_value) != 1) { + g_autofree long long *pid_value = g_new0(long long, 1);
I would rather use gint64 here as the exact type of gint64 changes with the hardware. For example on my AMD x86_84 it is 'signed long'.
If you use gint64, then you need to start using PRIu64 macro to deal with the fact that the data type changes for printf formatting.
Using long long is simpler as you can unconditionally use %ll which is a good thing IMHO.
Yup, simplicity of using %ll along with the fact that 'long long' is 64 bit signed integer on all of our supported platform lead me to use it without trying to get into the glib types.

On Thu, Oct 22, 2020 at 02:17:01PM +0100, Daniel P. Berrangé wrote:
On Thu, Oct 22, 2020 at 03:11:59PM +0200, Pavel Hrdina wrote:
On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
Rewrite using GHashTable which already has interfaces for using a number as hash key. Glib's implementation doesn't copy the key by default, so we need to allocate it, but overal the interface is more suited for this case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 61 ++++++++----------------------------- src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 2 +- src/util/vircgroupv2.c | 2 +- 5 files changed, 17 insertions(+), 53 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5f4cb01bc0..b74ec1a8fa 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -42,7 +42,6 @@ #include "virlog.h" #include "virfile.h" #include "virhash.h" -#include "virhashcode.h" #include "virstring.h" #include "virsystemd.h" #include "virtypedparam.h" @@ -2382,7 +2381,7 @@ virCgroupRemove(virCgroupPtr group) static int virCgroupKillInternal(virCgroupPtr group, int signum, - virHashTablePtr pids, + GHashTable *pids, int controller, const char *taskFile) { @@ -2415,8 +2414,9 @@ virCgroupKillInternal(virCgroupPtr group, goto cleanup; } else { while (!feof(fp)) { - long pid_value; - if (fscanf(fp, "%ld", &pid_value) != 1) { + g_autofree long long *pid_value = g_new0(long long, 1);
I would rather use gint64 here as the exact type of gint64 changes with the hardware. For example on my AMD x86_84 it is 'signed long'.
If you use gint64, then you need to start using PRIu64 macro to deal with the fact that the data type changes for printf formatting.
Using long long is simpler as you can unconditionally use %ll which is a good thing IMHO.
We should do this every time we pass pointers to GLib APIs because for example bool and gboolean are different and when I used bool type in GLib dbus APIs it randomly crashed.
bool vs gboolean is a special case because of the different types.
For all the other g* basic types, we should not use them. GLib has a ticket open considering deprecating them, because they re-invent stdint.h
Good to know. I personally don't like these types as it complicates thinks especially with the gboolean which is not that obvious. I checked the GLib code and it handles it reasonably so using long long should not be an issue. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Remove 'cleanup' label and simplify remembering of the returned value from the callback. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b74ec1a8fa..dafc6c98c0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2532,8 +2532,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, int virCgroupKillRecursive(virCgroupPtr group, int signum) { - int ret = 0; - int rc; + int ret = -1; size_t i; bool backendAvailable = false; virCgroupBackendPtr *backends = virCgroupBackendGetAll(); @@ -2544,24 +2543,17 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { if (backends && backends[i] && backends[i]->available()) { backendAvailable = true; - rc = backends[i]->killRecursive(group, signum, pids); - if (rc < 0) { - ret = -1; - goto cleanup; - } - if (rc > 0) - ret = rc; + if ((ret = backends[i]->killRecursive(group, signum, pids)) < 0) + return -1; } } if (!backends || !backendAvailable) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no cgroup backend available")); - ret = -1; - goto cleanup; + return -1; } - cleanup: return ret; } -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:55AM +0200, Peter Krempa wrote:
Remove 'cleanup' label and simplify remembering of the returned value from the callback.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b74ec1a8fa..dafc6c98c0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2532,8 +2532,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, int virCgroupKillRecursive(virCgroupPtr group, int signum) { - int ret = 0; - int rc; + int ret = -1; size_t i; bool backendAvailable = false; virCgroupBackendPtr *backends = virCgroupBackendGetAll(); @@ -2544,24 +2543,17 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { if (backends && backends[i] && backends[i]->available()) { backendAvailable = true; - rc = backends[i]->killRecursive(group, signum, pids); - if (rc < 0) { - ret = -1; - goto cleanup; - } - if (rc > 0) - ret = rc; + if ((ret = backends[i]->killRecursive(group, signum, pids)) < 0) + return -1;
This doesn't look correct. In case that both cgroups v1 and v2 are used the first call could return 1 meaning that it managed to kill some process but the second call would probably return 0 because the process would be already gone.
} }
if (!backends || !backendAvailable) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no cgroup backend available")); - ret = -1; - goto cleanup; + return -1; }
- cleanup: return ret; }
-- 2.26.2

On Thu, Oct 22, 2020 at 11:50:01 +0200, Pavel Hrdina wrote:
On Thu, Oct 22, 2020 at 11:34:55AM +0200, Peter Krempa wrote:
Remove 'cleanup' label and simplify remembering of the returned value from the callback.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b74ec1a8fa..dafc6c98c0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2532,8 +2532,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, int virCgroupKillRecursive(virCgroupPtr group, int signum) { - int ret = 0; - int rc; + int ret = -1; size_t i; bool backendAvailable = false; virCgroupBackendPtr *backends = virCgroupBackendGetAll(); @@ -2544,24 +2543,17 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { if (backends && backends[i] && backends[i]->available()) { backendAvailable = true; - rc = backends[i]->killRecursive(group, signum, pids); - if (rc < 0) { - ret = -1; - goto cleanup; - } - if (rc > 0) - ret = rc; + if ((ret = backends[i]->killRecursive(group, signum, pids)) < 0) + return -1;
This doesn't look correct. In case that both cgroups v1 and v2 are used the first call could return 1 meaning that it managed to kill some process but the second call would probably return 0 because the process would be already gone.
Does it in such case even make sense to call the second callback? If yes, then I'll probably rather change it such, that a boolean variable will be set to true if any of the callbacks returns 1 to make it more obvious.

On Thu, Oct 22, 2020 at 11:55:06AM +0200, Peter Krempa wrote:
On Thu, Oct 22, 2020 at 11:50:01 +0200, Pavel Hrdina wrote:
On Thu, Oct 22, 2020 at 11:34:55AM +0200, Peter Krempa wrote:
Remove 'cleanup' label and simplify remembering of the returned value from the callback.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b74ec1a8fa..dafc6c98c0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2532,8 +2532,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, int virCgroupKillRecursive(virCgroupPtr group, int signum) { - int ret = 0; - int rc; + int ret = -1; size_t i; bool backendAvailable = false; virCgroupBackendPtr *backends = virCgroupBackendGetAll(); @@ -2544,24 +2543,17 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { if (backends && backends[i] && backends[i]->available()) { backendAvailable = true; - rc = backends[i]->killRecursive(group, signum, pids); - if (rc < 0) { - ret = -1; - goto cleanup; - } - if (rc > 0) - ret = rc; + if ((ret = backends[i]->killRecursive(group, signum, pids)) < 0) + return -1;
This doesn't look correct. In case that both cgroups v1 and v2 are used the first call could return 1 meaning that it managed to kill some process but the second call would probably return 0 because the process would be already gone.
Does it in such case even make sense to call the second callback?
If yes, then I'll probably rather change it such, that a boolean variable will be set to true if any of the callbacks returns 1 to make it more obvious.
Good question, if the list of processes is the same in cgroups v1 and v2 it should be enough to call it only for one the cgroups, but I would rather call it for both just to be on a safe side. Using boolean sounds good. Pavel

Remove 'cleanup' label and simplify remembering of the returned value from the callback. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b74ec1a8fa..b1caf11f47 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2532,8 +2532,8 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, int virCgroupKillRecursive(virCgroupPtr group, int signum) { - int ret = 0; int rc; + bool success = false; size_t i; bool backendAvailable = false; virCgroupBackendPtr *backends = virCgroupBackendGetAll(); @@ -2544,25 +2544,24 @@ virCgroupKillRecursive(virCgroupPtr group, int signum) for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { if (backends && backends[i] && backends[i]->available()) { backendAvailable = true; - rc = backends[i]->killRecursive(group, signum, pids); - if (rc < 0) { - ret = -1; - goto cleanup; - } + if ((rc = backends[i]->killRecursive(group, signum, pids)) < 0) + return -1; + if (rc > 0) - ret = rc; + success = true; } } + if (success) + return 1; + if (!backends || !backendAvailable) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no cgroup backend available")); - ret = -1; - goto cleanup; + return -1; } - cleanup: - return ret; + return 0; } -- 2.26.2

On Thu, Oct 22, 2020 at 02:29:49PM +0200, Peter Krempa wrote:
Remove 'cleanup' label and simplify remembering of the returned value from the callback.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Rewrite using GHashTable which already has interfaces for using a number as hash key. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 123 +++++++++-------------------------------- src/conf/domain_addr.h | 4 +- 2 files changed, 27 insertions(+), 100 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index c7419916aa..4e47c2547c 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -25,17 +25,18 @@ #include "virlog.h" #include "virstring.h" #include "domain_addr.h" -#include "virhashcode.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN VIR_LOG_INIT("conf.domain_addr"); static int -virDomainZPCIAddressReserveId(virHashTablePtr set, +virDomainZPCIAddressReserveId(GHashTable *set, virZPCIDeviceAddressID *id, const char *name) { + int *idval; + if (!id->isSet) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No zPCI %s to reserve"), @@ -43,26 +44,24 @@ virDomainZPCIAddressReserveId(virHashTablePtr set, return -1; } - if (virHashLookup(set, &id->value)) { + if (g_hash_table_lookup(set, &id->value)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("zPCI %s %o is already reserved"), name, id->value); return -1; } - if (virHashAddEntry(set, &id->value, (void*)1) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to reserve %s %o"), - name, id->value); - return -1; - } + idval = g_new0(int, 1); + *idval = (int) id->value; + + g_hash_table_add(set, idval); return 0; } static int -virDomainZPCIAddressReserveUid(virHashTablePtr set, +virDomainZPCIAddressReserveUid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressReserveId(set, &addr->uid, "uid"); @@ -70,7 +69,7 @@ virDomainZPCIAddressReserveUid(virHashTablePtr set, static int -virDomainZPCIAddressReserveFid(virHashTablePtr set, +virDomainZPCIAddressReserveFid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressReserveId(set, &addr->fid, "fid"); @@ -78,7 +77,7 @@ virDomainZPCIAddressReserveFid(virHashTablePtr set, static int -virDomainZPCIAddressAssignId(virHashTablePtr set, +virDomainZPCIAddressAssignId(GHashTable *set, virZPCIDeviceAddressID *id, unsigned int min, unsigned int max, @@ -87,7 +86,7 @@ virDomainZPCIAddressAssignId(virHashTablePtr set, if (id->isSet) return 0; - while (virHashLookup(set, &min)) { + while (g_hash_table_lookup(set, &min)) { if (min == max) { virReportError(VIR_ERR_INTERNAL_ERROR, _("There is no more free %s."), @@ -105,7 +104,7 @@ virDomainZPCIAddressAssignId(virHashTablePtr set, static int -virDomainZPCIAddressAssignUid(virHashTablePtr set, +virDomainZPCIAddressAssignUid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressAssignId(set, &addr->uid, 1, @@ -114,7 +113,7 @@ virDomainZPCIAddressAssignUid(virHashTablePtr set, static int -virDomainZPCIAddressAssignFid(virHashTablePtr set, +virDomainZPCIAddressAssignFid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressAssignId(set, &addr->fid, 0, @@ -123,40 +122,19 @@ virDomainZPCIAddressAssignFid(virHashTablePtr set, static void -virDomainZPCIAddressReleaseId(virHashTablePtr set, - virZPCIDeviceAddressID *id, - const char *name) +virDomainZPCIAddressReleaseId(GHashTable *set, + virZPCIDeviceAddressID *id) { if (!id->isSet) return; - if (virHashRemoveEntry(set, &id->value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Release %s %o failed"), - name, id->value); - } + g_hash_table_remove(set, &id->value); id->value = 0; id->isSet = false; } -static void -virDomainZPCIAddressReleaseUid(virHashTablePtr set, - virZPCIDeviceAddressPtr addr) -{ - virDomainZPCIAddressReleaseId(set, &addr->uid, "uid"); -} - - -static void -virDomainZPCIAddressReleaseFid(virHashTablePtr set, - virZPCIDeviceAddressPtr addr) -{ - virDomainZPCIAddressReleaseId(set, &addr->fid, "fid"); -} - - static void virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds, virZPCIDeviceAddressPtr addr) @@ -164,8 +142,8 @@ virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds, if (!zpciIds) return; - virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); - virDomainZPCIAddressReleaseFid(zpciIds->fids, addr); + virDomainZPCIAddressReleaseId(zpciIds->uids, &addr->uid); + virDomainZPCIAddressReleaseId(zpciIds->fids, &addr->fid); } @@ -183,7 +161,7 @@ virDomainZPCIAddressEnsureAddr(virDomainZPCIAddressIdsPtr zpciIds, return -1; if (virDomainZPCIAddressReserveFid(zpciIds->fids, addr) < 0) { - virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); + virDomainZPCIAddressReleaseId(zpciIds->uids, &addr->uid); return -1; } @@ -965,55 +943,15 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, } -static uint32_t -virZPCIAddrKeyCode(const void *name, - uint32_t seed) -{ - unsigned int value = *((unsigned int *)name); - return virHashCodeGen(&value, sizeof(value), seed); -} - - -static bool -virZPCIAddrKeyEqual(const void *namea, - const void *nameb) -{ - return *((unsigned int *)namea) == *((unsigned int *)nameb); -} - - -static void * -virZPCIAddrKeyCopy(const void *name) -{ - unsigned int *copy = g_new0(unsigned int, 1); - - *copy = *((unsigned int *)name); - return (void *)copy; -} - - -static char * -virZPCIAddrKeyPrintHuman(const void *name) -{ - return g_strdup_printf("%u", *((unsigned int *)name)); -} - - -static void -virZPCIAddrKeyFree(void *name) -{ - VIR_FREE(name); -} - - static void virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs) { if (!addrs || !addrs->zpciIds) return; - virHashFree(addrs->zpciIds->uids); - virHashFree(addrs->zpciIds->fids); + g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref); + g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref); + VIR_FREE(addrs->zpciIds); } @@ -1028,19 +966,8 @@ virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs, addrs->zpciIds = g_new0(virDomainZPCIAddressIds, 1); - addrs->zpciIds->uids = virHashCreateFull(10, NULL, - virZPCIAddrKeyCode, - virZPCIAddrKeyEqual, - virZPCIAddrKeyCopy, - virZPCIAddrKeyPrintHuman, - virZPCIAddrKeyFree); - - addrs->zpciIds->fids = virHashCreateFull(10, NULL, - virZPCIAddrKeyCode, - virZPCIAddrKeyEqual, - virZPCIAddrKeyCopy, - virZPCIAddrKeyPrintHuman, - virZPCIAddrKeyFree); + addrs->zpciIds->uids = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, NULL); + addrs->zpciIds->fids = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, NULL); } return 0; diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index a0460b4030..77dd091fd3 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -115,8 +115,8 @@ typedef struct { typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr; typedef struct { - virHashTablePtr uids; - virHashTablePtr fids; + GHashTable *uids; + GHashTable *fids; } virDomainZPCIAddressIds; typedef virDomainZPCIAddressIds *virDomainZPCIAddressIdsPtr; -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:56AM +0200, Peter Krempa wrote:
Rewrite using GHashTable which already has interfaces for using a number as hash key.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 123 +++++++++-------------------------------- src/conf/domain_addr.h | 4 +- 2 files changed, 27 insertions(+), 100 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Peter Krempa <pkrempa@redhat.com> [2020-10-22, 11:34AM +0200]:
Rewrite using GHashTable which already has interfaces for using a number as hash key.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 123 +++++++++-------------------------------- src/conf/domain_addr.h | 4 +- 2 files changed, 27 insertions(+), 100 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index c7419916aa..4e47c2547c 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -25,17 +25,18 @@ #include "virlog.h" #include "virstring.h" #include "domain_addr.h" -#include "virhashcode.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
VIR_LOG_INIT("conf.domain_addr");
static int -virDomainZPCIAddressReserveId(virHashTablePtr set, +virDomainZPCIAddressReserveId(GHashTable *set, virZPCIDeviceAddressID *id, const char *name) { + int *idval; + if (!id->isSet) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No zPCI %s to reserve"), @@ -43,26 +44,24 @@ virDomainZPCIAddressReserveId(virHashTablePtr set, return -1; }
- if (virHashLookup(set, &id->value)) { + if (g_hash_table_lookup(set, &id->value)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("zPCI %s %o is already reserved"), name, id->value); return -1; }
- if (virHashAddEntry(set, &id->value, (void*)1) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to reserve %s %o"), - name, id->value); - return -1; - } + idval = g_new0(int, 1); + *idval = (int) id->value; + + g_hash_table_add(set, idval);
g_hash_table_add() can't fail, only abort(), right? Then dropping the error message should be fine.
return 0; }
static int -virDomainZPCIAddressReserveUid(virHashTablePtr set, +virDomainZPCIAddressReserveUid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressReserveId(set, &addr->uid, "uid"); @@ -70,7 +69,7 @@ virDomainZPCIAddressReserveUid(virHashTablePtr set,
static int -virDomainZPCIAddressReserveFid(virHashTablePtr set, +virDomainZPCIAddressReserveFid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressReserveId(set, &addr->fid, "fid"); @@ -78,7 +77,7 @@ virDomainZPCIAddressReserveFid(virHashTablePtr set,
static int -virDomainZPCIAddressAssignId(virHashTablePtr set, +virDomainZPCIAddressAssignId(GHashTable *set, virZPCIDeviceAddressID *id, unsigned int min, unsigned int max, @@ -87,7 +86,7 @@ virDomainZPCIAddressAssignId(virHashTablePtr set, if (id->isSet) return 0;
- while (virHashLookup(set, &min)) { + while (g_hash_table_lookup(set, &min)) { if (min == max) { virReportError(VIR_ERR_INTERNAL_ERROR, _("There is no more free %s."), @@ -105,7 +104,7 @@ virDomainZPCIAddressAssignId(virHashTablePtr set,
static int -virDomainZPCIAddressAssignUid(virHashTablePtr set, +virDomainZPCIAddressAssignUid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressAssignId(set, &addr->uid, 1, @@ -114,7 +113,7 @@ virDomainZPCIAddressAssignUid(virHashTablePtr set,
static int -virDomainZPCIAddressAssignFid(virHashTablePtr set, +virDomainZPCIAddressAssignFid(GHashTable *set, virZPCIDeviceAddressPtr addr) { return virDomainZPCIAddressAssignId(set, &addr->fid, 0, @@ -123,40 +122,19 @@ virDomainZPCIAddressAssignFid(virHashTablePtr set,
static void -virDomainZPCIAddressReleaseId(virHashTablePtr set, - virZPCIDeviceAddressID *id, - const char *name) +virDomainZPCIAddressReleaseId(GHashTable *set, + virZPCIDeviceAddressID *id) { if (!id->isSet) return;
- if (virHashRemoveEntry(set, &id->value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Release %s %o failed"), - name, id->value); - } + g_hash_table_remove(set, &id->value);
Here I'm not so sure, IIUC the original code reported an error when the value was not found in the hash table. This will be dropped with the new code. But since this should only occur in pathological cases anyways, so I guess this is fine.
id->value = 0; id->isSet = false; }
-static void -virDomainZPCIAddressReleaseUid(virHashTablePtr set, - virZPCIDeviceAddressPtr addr) -{ - virDomainZPCIAddressReleaseId(set, &addr->uid, "uid"); -} - - -static void -virDomainZPCIAddressReleaseFid(virHashTablePtr set, - virZPCIDeviceAddressPtr addr) -{ - virDomainZPCIAddressReleaseId(set, &addr->fid, "fid"); -} - - static void virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds, virZPCIDeviceAddressPtr addr) @@ -164,8 +142,8 @@ virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds, if (!zpciIds) return;
- virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); - virDomainZPCIAddressReleaseFid(zpciIds->fids, addr); + virDomainZPCIAddressReleaseId(zpciIds->uids, &addr->uid); + virDomainZPCIAddressReleaseId(zpciIds->fids, &addr->fid); }
@@ -183,7 +161,7 @@ virDomainZPCIAddressEnsureAddr(virDomainZPCIAddressIdsPtr zpciIds, return -1;
if (virDomainZPCIAddressReserveFid(zpciIds->fids, addr) < 0) { - virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); + virDomainZPCIAddressReleaseId(zpciIds->uids, &addr->uid); return -1; }
@@ -965,55 +943,15 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, }
-static uint32_t -virZPCIAddrKeyCode(const void *name, - uint32_t seed) -{ - unsigned int value = *((unsigned int *)name); - return virHashCodeGen(&value, sizeof(value), seed); -} - - -static bool -virZPCIAddrKeyEqual(const void *namea, - const void *nameb) -{ - return *((unsigned int *)namea) == *((unsigned int *)nameb); -} - - -static void * -virZPCIAddrKeyCopy(const void *name) -{ - unsigned int *copy = g_new0(unsigned int, 1); - - *copy = *((unsigned int *)name); - return (void *)copy; -} - - -static char * -virZPCIAddrKeyPrintHuman(const void *name) -{ - return g_strdup_printf("%u", *((unsigned int *)name)); -} - - -static void -virZPCIAddrKeyFree(void *name) -{ - VIR_FREE(name); -} - - static void virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs) { if (!addrs || !addrs->zpciIds) return;
- virHashFree(addrs->zpciIds->uids); - virHashFree(addrs->zpciIds->fids); + g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref); + g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref); + VIR_FREE(addrs->zpciIds); }
@@ -1028,19 +966,8 @@ virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
addrs->zpciIds = g_new0(virDomainZPCIAddressIds, 1);
- addrs->zpciIds->uids = virHashCreateFull(10, NULL, - virZPCIAddrKeyCode, - virZPCIAddrKeyEqual, - virZPCIAddrKeyCopy, - virZPCIAddrKeyPrintHuman, - virZPCIAddrKeyFree); - - addrs->zpciIds->fids = virHashCreateFull(10, NULL, - virZPCIAddrKeyCode, - virZPCIAddrKeyEqual, - virZPCIAddrKeyCopy, - virZPCIAddrKeyPrintHuman, - virZPCIAddrKeyFree); + addrs->zpciIds->uids = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, NULL); + addrs->zpciIds->fids = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, NULL); }
return 0; diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index a0460b4030..77dd091fd3 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -115,8 +115,8 @@ typedef struct { typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr;
typedef struct { - virHashTablePtr uids; - virHashTablePtr fids; + GHashTable *uids; + GHashTable *fids; } virDomainZPCIAddressIds; typedef virDomainZPCIAddressIds *virDomainZPCIAddressIdsPtr;
-- 2.26.2
Rest looks good. Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> -- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Thu, Oct 22, 2020 at 13:44:10 +0200, Bjoern Walk wrote:
Peter Krempa <pkrempa@redhat.com> [2020-10-22, 11:34AM +0200]:
Rewrite using GHashTable which already has interfaces for using a number as hash key.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 123 +++++++++-------------------------------- src/conf/domain_addr.h | 4 +- 2 files changed, 27 insertions(+), 100 deletions(-)
[...]
@@ -123,40 +122,19 @@ virDomainZPCIAddressAssignFid(virHashTablePtr set,
static void -virDomainZPCIAddressReleaseId(virHashTablePtr set, - virZPCIDeviceAddressID *id, - const char *name) +virDomainZPCIAddressReleaseId(GHashTable *set, + virZPCIDeviceAddressID *id) { if (!id->isSet) return;
- if (virHashRemoveEntry(set, &id->value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Release %s %o failed"), - name, id->value); - } + g_hash_table_remove(set, &id->value);
Here I'm not so sure, IIUC the original code reported an error when the value was not found in the hash table. This will be dropped with the new code. But since this should only occur in pathological cases anyways, so
Specifically, here the error is only logged, but the caller doesn't take any action/return any value. That means that there wouldn't be any immediate notification of the user. It's very unlikely that the error will be noticed in logs. g_hash_table_remove returns whether it released the value or not, but I don't feel it's worth keeping it.
I guess this is fine.
Thanks!

Use 'virHashNew' internally which uses a default size. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 2 +- src/util/virhash.c | 5 ++--- src/util/virhash.h | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e18216824c..2f5d61f8e7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5743,7 +5743,7 @@ qemuMigrationDstErrorFree(void *data) int qemuMigrationDstErrorInit(virQEMUDriverPtr driver) { - driver->migrationErrors = virHashAtomicNew(64, qemuMigrationDstErrorFree); + driver->migrationErrors = virHashAtomicNew(qemuMigrationDstErrorFree); if (driver->migrationErrors) return 0; else diff --git a/src/util/virhash.c b/src/util/virhash.c index 7dd2d9f81d..23e12e0255 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -212,8 +212,7 @@ virHashTablePtr virHashCreate(ssize_t size, virHashDataFree dataFree) virHashAtomicPtr -virHashAtomicNew(ssize_t size, - virHashDataFree dataFree) +virHashAtomicNew(virHashDataFree dataFree) { virHashAtomicPtr hash; @@ -223,7 +222,7 @@ virHashAtomicNew(ssize_t size, if (!(hash = virObjectLockableNew(virHashAtomicClass))) return NULL; - if (!(hash->hash = virHashCreate(size, dataFree))) { + if (!(hash->hash = virHashNew(dataFree))) { virObjectUnref(hash); return NULL; } diff --git a/src/util/virhash.h b/src/util/virhash.h index 37853aba36..baf92996a3 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -114,8 +114,7 @@ typedef void (*virHashKeyFree)(void *name); virHashTablePtr virHashNew(virHashDataFree dataFree); virHashTablePtr virHashCreate(ssize_t size, virHashDataFree dataFree); -virHashAtomicPtr virHashAtomicNew(ssize_t size, - virHashDataFree dataFree); +virHashAtomicPtr virHashAtomicNew(virHashDataFree dataFree); virHashTablePtr virHashCreateFull(ssize_t size, virHashDataFree dataFree, virHashKeyCode keyCode, -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:57AM +0200, Peter Krempa wrote:
Use 'virHashNew' internally which uses a default size.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 2 +- src/util/virhash.c | 5 ++--- src/util/virhash.h | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Export the freeing function rather than having a wrapper for the hash creation function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_nwfilter.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 2 +- src/conf/nwfilter_params.c | 12 +++--------- src/conf/nwfilter_params.h | 2 +- src/conf/virnwfilterbindingdef.c | 2 +- src/libvirt_private.syms | 2 +- src/nwfilter/nwfilter_gentech_driver.c | 6 +++--- tests/nwfilterxml2firewalltest.c | 6 +++--- 8 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index ef388d5d02..f32122b8a3 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -59,7 +59,7 @@ virNWFilterBindingDefForNet(const char *vmname, ret->filter = g_strdup(net->filter); - if (!(ret->filterparams = virNWFilterHashTableCreate(0))) + if (!(ret->filterparams = virHashNew(virNWFilterVarValueHashFree))) goto error; if (net->filterparams && diff --git a/src/conf/nwfilter_ipaddrmap.c b/src/conf/nwfilter_ipaddrmap.c index 672a58be3a..487e7f22bd 100644 --- a/src/conf/nwfilter_ipaddrmap.c +++ b/src/conf/nwfilter_ipaddrmap.c @@ -149,7 +149,7 @@ virNWFilterIPAddrMapGetIPAddr(const char *ifname) int virNWFilterIPAddrMapInit(void) { - ipAddressMap = virNWFilterHashTableCreate(0); + ipAddressMap = virHashNew(virNWFilterVarValueHashFree); if (!ipAddressMap) return -1; diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 1210079954..fd05b45ca3 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -606,19 +606,13 @@ virNWFilterVarCombIterGetVarValue(virNWFilterVarCombIterPtr ci, return res; } -static void -hashDataFree(void *payload) +void +virNWFilterVarValueHashFree(void *payload) { virNWFilterVarValueFree(payload); } -virHashTablePtr -virNWFilterHashTableCreate(int n) -{ - return virHashCreate(n, hashDataFree); -} - struct addToTableStruct { virHashTablePtr target; int errOccurred; @@ -708,7 +702,7 @@ virNWFilterParseParamAttributes(xmlNodePtr cur) char *nam, *val; virNWFilterVarValuePtr value; - virHashTablePtr table = virNWFilterHashTableCreate(0); + virHashTablePtr table = virHashNew(virNWFilterVarValueHashFree); if (!table) return NULL; diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h index d51f3f7f9f..4962a86864 100644 --- a/src/conf/nwfilter_params.h +++ b/src/conf/nwfilter_params.h @@ -52,6 +52,7 @@ virNWFilterVarValuePtr virNWFilterVarValueCreateSimple(char *); virNWFilterVarValuePtr virNWFilterVarValueCreateSimpleCopyValue(const char *); virNWFilterVarValuePtr virNWFilterVarValueCopy(const virNWFilterVarValue *); void virNWFilterVarValueFree(virNWFilterVarValuePtr val); +void virNWFilterVarValueHashFree(void *payload); const char *virNWFilterVarValueGetSimple(const virNWFilterVarValue *val); const char *virNWFilterVarValueGetNthValue(const virNWFilterVarValue *val, unsigned int idx); @@ -67,7 +68,6 @@ int virNWFilterFormatParamAttributes(virBufferPtr buf, virHashTablePtr table, const char *filterref); -virHashTablePtr virNWFilterHashTableCreate(int n); int virNWFilterHashTablePutAll(virHashTablePtr src, virHashTablePtr dest); bool virNWFilterHashTableEqual(virHashTablePtr a, diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 81f8e72182..28c1c3938c 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -65,7 +65,7 @@ virNWFilterBindingDefCopy(virNWFilterBindingDefPtr src) ret->filter = g_strdup(src->filter); - if (!(ret->filterparams = virNWFilterHashTableCreate(0))) + if (!(ret->filterparams = virHashNew(virNWFilterVarValueHashFree))) goto error; if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 79c8403208..4f9e7f6048 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -927,7 +927,6 @@ virNWFilterIPAddrMapShutdown; # conf/nwfilter_params.h -virNWFilterHashTableCreate; virNWFilterHashTableEqual; virNWFilterHashTablePutAll; virNWFilterVarAccessGetVarName; @@ -948,6 +947,7 @@ virNWFilterVarValueFree; virNWFilterVarValueGetCardinality; virNWFilterVarValueGetNthValue; virNWFilterVarValueGetSimple; +virNWFilterVarValueHashFree; # conf/object_event.h diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index c93f2ed18f..99d2c0fce4 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -199,7 +199,7 @@ static virHashTablePtr virNWFilterCreateVarsFrom(virHashTablePtr vars1, virHashTablePtr vars2) { - virHashTablePtr res = virNWFilterHashTableCreate(0); + virHashTablePtr res = virHashNew(virNWFilterVarValueHashFree); if (!res) return NULL; @@ -268,7 +268,7 @@ virNWFilterRuleDefToRuleInst(virNWFilterDefPtr def, ruleinst->chainPriority = def->chainPriority; ruleinst->def = rule; ruleinst->priority = rule->priority; - if (!(ruleinst->vars = virNWFilterHashTableCreate(0))) + if (!(ruleinst->vars = virHashNew(virNWFilterVarValueHashFree))) goto cleanup; if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0) goto cleanup; @@ -516,7 +516,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver, const char *learning; bool reportIP = false; - virHashTablePtr missing_vars = virNWFilterHashTableCreate(0); + virHashTablePtr missing_vars = virHashNew(virNWFilterVarValueHashFree); memset(&inst, 0, sizeof(inst)); diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 755732487a..30f3dab97e 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -149,7 +149,7 @@ static virHashTablePtr virNWFilterCreateVarsFrom(virHashTablePtr vars1, virHashTablePtr vars2) { - virHashTablePtr res = virNWFilterHashTableCreate(0); + virHashTablePtr res = virHashNew(virNWFilterVarValueHashFree); if (!res) return NULL; @@ -215,7 +215,7 @@ virNWFilterRuleDefToRuleInst(virNWFilterDefPtr def, ruleinst->chainPriority = def->chainPriority; ruleinst->def = rule; ruleinst->priority = rule->priority; - if (!(ruleinst->vars = virNWFilterHashTableCreate(0))) + if (!(ruleinst->vars = virHashNew(virNWFilterVarValueHashFree))) goto cleanup; if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0) goto cleanup; @@ -368,7 +368,7 @@ static int testCompareXMLToArgvFiles(const char *xml, { char *actualargv = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - virHashTablePtr vars = virNWFilterHashTableCreate(0); + virHashTablePtr vars = virHashNew(virNWFilterVarValueHashFree); virNWFilterInst inst; int ret = -1; -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:58AM +0200, Peter Krempa wrote:
Export the freeing function rather than having a wrapper for the hash creation function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_nwfilter.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 2 +- src/conf/nwfilter_params.c | 12 +++--------- src/conf/nwfilter_params.h | 2 +- src/conf/virnwfilterbindingdef.c | 2 +- src/libvirt_private.syms | 2 +- src/nwfilter/nwfilter_gentech_driver.c | 6 +++--- tests/nwfilterxml2firewalltest.c | 6 +++--- 8 files changed, 14 insertions(+), 20 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Most callers pass a random number. We have virHashNew which doesn't give the callers the option to configure the table. Since we are going to switch to virHashNew replace it in tests and remove multiple instances of the 'testHashGrow' case as it doesn't make sense with the new semantics. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virhashtest.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 7b1b8dd38c..bc93c07d1f 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -15,12 +15,12 @@ VIR_LOG_INIT("tests.hashtest"); static virHashTablePtr -testHashInit(int size) +testHashInit(void) { virHashTablePtr hash; ssize_t i; - if (!(hash = virHashCreate(size, NULL))) + if (!(hash = virHashNew(NULL))) return NULL; /* entries are added in reverse order so that they will be linked in @@ -83,13 +83,12 @@ struct testInfo { static int -testHashGrow(const void *data) +testHashGrow(const void *data G_GNUC_UNUSED) { - const struct testInfo *info = data; virHashTablePtr hash; int ret = -1; - if (!(hash = testHashInit(info->count))) + if (!(hash = testHashInit())) return -1; if (testHashCheckCount(hash, G_N_ELEMENTS(uuids)) < 0) @@ -111,7 +110,7 @@ testHashUpdate(const void *data G_GNUC_UNUSED) size_t i; int ret = -1; - if (!(hash = testHashInit(0))) + if (!(hash = testHashInit())) return -1; for (i = 0; i < G_N_ELEMENTS(uuids_subset); i++) { @@ -149,7 +148,7 @@ testHashRemove(const void *data G_GNUC_UNUSED) size_t i; int ret = -1; - if (!(hash = testHashInit(0))) + if (!(hash = testHashInit())) return -1; for (i = 0; i < G_N_ELEMENTS(uuids_subset); i++) { @@ -216,7 +215,7 @@ testHashRemoveForEach(const void *data) virHashTablePtr hash; int ret = -1; - if (!(hash = testHashInit(0))) + if (!(hash = testHashInit())) return -1; if (virHashForEach(hash, (virHashIterator) info->data, hash)) { @@ -243,7 +242,7 @@ testHashSteal(const void *data G_GNUC_UNUSED) size_t i; int ret = -1; - if (!(hash = testHashInit(0))) + if (!(hash = testHashInit())) return -1; for (i = 0; i < G_N_ELEMENTS(uuids_subset); i++) { @@ -297,7 +296,7 @@ testHashRemoveSet(const void *data G_GNUC_UNUSED) int rcount; int ret = -1; - if (!(hash = testHashInit(0))) + if (!(hash = testHashInit())) return -1; /* seed the generator so that rand() provides reproducible sequence */ @@ -340,7 +339,7 @@ testHashSearch(const void *data G_GNUC_UNUSED) void *entry; int ret = -1; - if (!(hash = testHashInit(0))) + if (!(hash = testHashInit())) return -1; entry = virHashSearch(hash, testHashSearchIter, NULL, NULL); @@ -544,15 +543,10 @@ mymain(void) testHash ## cmd ## data, \ testHashCount ## cmd ## data) -#define DO_TEST_COUNT(name, cmd, count) \ - DO_TEST_FULL(name "(" #count ")", cmd, NULL, count) - #define DO_TEST(name, cmd) \ DO_TEST_FULL(name, cmd, NULL, -1) - DO_TEST_COUNT("Grow", Grow, 1); - DO_TEST_COUNT("Grow", Grow, 10); - DO_TEST_COUNT("Grow", Grow, 42); + DO_TEST("Grow", Grow); DO_TEST("Update", Update); DO_TEST("Remove", Remove); DO_TEST_DATA("Remove in ForEach", RemoveForEach, Some); -- 2.26.2

On Thu, Oct 22, 2020 at 11:34:59AM +0200, Peter Krempa wrote:
Most callers pass a random number. We have virHashNew which doesn't give the callers the option to configure the table. Since we are going to switch to virHashNew replace it in tests and remove multiple instances of the 'testHashGrow' case as it doesn't make sense with the new semantics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virhashtest.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

virHashCreate will be removed in upcoming patches. This change has an impact on ordering of the blockjob entries in one of the status XML->XML tests. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 +- .../blockjob-blockdev-in.xml | 110 +++++++++--------- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index eeceb44348..bea43a1aba 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1697,7 +1697,7 @@ qemuDomainObjPrivateAlloc(void *opaque) if (!(priv->devs = virChrdevAlloc())) goto error; - if (!(priv->blockjobs = virHashCreate(5, virObjectFreeHashData))) + if (!(priv->blockjobs = virHashNew(virObjectFreeHashData))) goto error; /* agent commands block by default, user can choose different behavior */ diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index ca6d110179..8ffc91bdcb 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -244,64 +244,9 @@ <top node='libvirt-17-format'/> <deleteCommittedImages/> </blockjob> - <blockjob name='create-libvirt-1337-storage' type='create' state='running'> - <create mode='storage'/> - <src type='network' format='qcow2'> - <source protocol='rbd' name='pool/volname.qcow2' tlsFromConfig='0' index='1337'> - <host name='example.org'/> - <privateData> - <nodenames> - <nodename type='storage' name='libvirt-1337-storage'/> - <nodename type='format' name='libvirt-1337-format'/> - </nodenames> - <objects> - <secret type='auth' alias='libvirt-1337-storage-secret0'/> - </objects> - </privateData> - </source> - </src> - </blockjob> - <blockjob name='copy-vdd-libvirt-4321-format' type='copy' state='ready' jobflags='0x0' shallownew='yes'> - <disk dst='vdd'/> - </blockjob> - <blockjob name='commit-vdc-libvirt-9-format' type='commit' state='running' jobflags='0x0'> - <disk dst='vdc'/> - <base node='libvirt-11-format'/> - <top node='libvirt-9-format'/> - <topparent node='libvirt-2-format'/> - </blockjob> <blockjob name='drive-virtio-disk0' type='copy' state='ready' jobflags='0x0'> <disk dst='vda' mirror='yes'/> </blockjob> - <blockjob name='create-libvirt-1338-format' type='create' state='running' jobflags='0xabcd'> - <chains> - <disk type='file' format='qcow2'> - <source file='/create/src1.qcow2' index='1339'> - <privateData> - <nodenames> - <nodename type='storage' name='libvirt-1339-storage'/> - <nodename type='format' name='libvirt-1339-format'/> - </nodenames> - </privateData> - </source> - <backingStore/> - </disk> - </chains> - <src type='file' format='qcow2'> - <source file='/tmp/create/overlay.qcow2' index='1338'> - <privateData> - <nodenames> - <nodename type='storage' name='libvirt-1338-storage'/> - <nodename type='format' name='libvirt-1338-format'/> - </nodenames> - <objects> - <secret type='encryption' alias='libvirt-1338-storage-secret0'/> - </objects> - </privateData> - </source> - </src> - </blockjob> - <blockjob name='broken-test' type='broken' state='ready' brokentype='commit'/> <blockjob name='test-orphan-job0' type='copy' state='ready'> <chains> <disk type='file' format='qcow2'> @@ -339,6 +284,61 @@ </mirror> </chains> </blockjob> + <blockjob name='broken-test' type='broken' state='ready' brokentype='commit'/> + <blockjob name='create-libvirt-1338-format' type='create' state='running' jobflags='0xabcd'> + <chains> + <disk type='file' format='qcow2'> + <source file='/create/src1.qcow2' index='1339'> + <privateData> + <nodenames> + <nodename type='storage' name='libvirt-1339-storage'/> + <nodename type='format' name='libvirt-1339-format'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </disk> + </chains> + <src type='file' format='qcow2'> + <source file='/tmp/create/overlay.qcow2' index='1338'> + <privateData> + <nodenames> + <nodename type='storage' name='libvirt-1338-storage'/> + <nodename type='format' name='libvirt-1338-format'/> + </nodenames> + <objects> + <secret type='encryption' alias='libvirt-1338-storage-secret0'/> + </objects> + </privateData> + </source> + </src> + </blockjob> + <blockjob name='commit-vdc-libvirt-9-format' type='commit' state='running' jobflags='0x0'> + <disk dst='vdc'/> + <base node='libvirt-11-format'/> + <top node='libvirt-9-format'/> + <topparent node='libvirt-2-format'/> + </blockjob> + <blockjob name='create-libvirt-1337-storage' type='create' state='running'> + <create mode='storage'/> + <src type='network' format='qcow2'> + <source protocol='rbd' name='pool/volname.qcow2' tlsFromConfig='0' index='1337'> + <host name='example.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='libvirt-1337-storage'/> + <nodename type='format' name='libvirt-1337-format'/> + </nodenames> + <objects> + <secret type='auth' alias='libvirt-1337-storage-secret0'/> + </objects> + </privateData> + </source> + </src> + </blockjob> + <blockjob name='copy-vdd-libvirt-4321-format' type='copy' state='ready' jobflags='0x0' shallownew='yes'> + <disk dst='vdd'/> + </blockjob> </blockjobs> <agentTimeout>-2</agentTimeout> <domain type='kvm' id='4'> -- 2.26.2

On Thu, Oct 22, 2020 at 11:35:00AM +0200, Peter Krempa wrote:
virHashCreate will be removed in upcoming patches. This change has an impact on ordering of the blockjob entries in one of the status XML->XML tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 +- .../blockjob-blockdev-in.xml | 110 +++++++++--------- 2 files changed, 56 insertions(+), 56 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

It doesn't make much sense to configure the bucket count in the hash table for each case specifically. Replace all calls of virHashCreate with virHashNew which has a pre-set size and remove virHashCreate completely. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 4 ++-- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 4 ++-- src/conf/virinterfaceobj.c | 2 +- src/conf/virnetworkobj.c | 5 ++--- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingobjlist.c | 2 +- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 10 +++++----- src/hyperv/hyperv_wmi.c | 2 +- src/hypervisor/virclosecallbacks.c | 2 +- src/libvirt_private.syms | 1 - src/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 8 ++------ src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++--- src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++-- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 4 ++-- src/qemu/qemu_block.c | 6 +++--- src/qemu/qemu_capabilities.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_interop_config.c | 2 +- src/qemu/qemu_monitor.c | 10 +++++----- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_qapi.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/security/security_selinux.c | 4 ++-- src/util/virfilecache.c | 2 +- src/util/virhash.c | 21 --------------------- src/util/virhash.h | 2 -- src/util/viriptables.c | 4 ++-- src/util/virlockspace.c | 6 ++---- src/util/virmacmap.c | 2 +- src/util/virstoragefile.c | 4 ++-- src/util/virsystemd.c | 2 +- tests/qemumonitorjsontest.c | 10 +++++----- tests/qemusecuritymock.c | 4 ++-- tests/testutilsqemu.c | 2 +- tests/virhashtest.c | 8 ++++---- 41 files changed, 69 insertions(+), 100 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 4e47c2547c..de344186ec 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1389,7 +1389,7 @@ virDomainCCWAddressSetCreate(void) addrs = g_new0(virDomainCCWAddressSet, 1); - if (!(addrs->defined = virHashCreate(10, virHashValueFree))) + if (!(addrs->defined = virHashNew(virHashValueFree))) goto error; /* must use cssid = 0xfe (254) for virtio-ccw devices */ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 37efd104f1..f4f017cf83 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5821,7 +5821,7 @@ virDomainDefBootOrderPostParse(virDomainDefPtr def) virHashTablePtr bootHash = NULL; int ret = -1; - if (!(bootHash = virHashCreate(5, NULL))) + if (!(bootHash = virHashNew(NULL))) goto cleanup; if (virDomainDeviceInfoIterate(def, virDomainDefCollectBootOrder, bootHash) < 0) @@ -6927,7 +6927,7 @@ virDomainDefValidateAliases(const virDomainDef *def, int ret = -1; /* We are not storing copies of aliases. Don't free them. */ - if (!(data.aliases = virHashCreate(10, NULL))) + if (!(data.aliases = virHashNew(NULL))) goto cleanup; if (virDomainDeviceInfoIterateInternal((virDomainDefPtr) def, diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index c50f9ca720..0d2c9503ab 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -266,7 +266,7 @@ virChrdevsPtr virChrdevAlloc(void) /* there will hardly be any devices most of the time, the hash * does not have to be huge */ - if (!(devs->hash = virHashCreate(3, virChrdevHashEntryFree))) + if (!(devs->hash = virHashNew(virChrdevHashEntryFree))) goto error; return devs; diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 8a1eb6b734..43c77e6c54 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -275,7 +275,7 @@ virDomainMomentObjListNew(void) virDomainMomentObjListPtr moments; moments = g_new0(virDomainMomentObjList, 1); - moments->objs = virHashCreate(50, virDomainMomentObjListDataFree); + moments->objs = virHashNew(virDomainMomentObjListDataFree); if (!moments->objs) { VIR_FREE(moments); return NULL; diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 46b28cc9a6..9e8757eff9 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -75,8 +75,8 @@ virDomainObjListPtr virDomainObjListNew(void) if (!(doms = virObjectRWLockableNew(virDomainObjListClass))) return NULL; - if (!(doms->objs = virHashCreate(50, virObjectFreeHashData)) || - !(doms->objsName = virHashCreate(50, virObjectFreeHashData))) { + if (!(doms->objs = virHashNew(virObjectFreeHashData)) || + !(doms->objsName = virHashNew(virObjectFreeHashData))) { virObjectUnref(doms); return NULL; } diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index e08b4b9b0f..1e29e12148 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -142,7 +142,7 @@ virInterfaceObjListNew(void) if (!(interfaces = virObjectRWLockableNew(virInterfaceObjListClass))) return NULL; - if (!(interfaces->objsName = virHashCreate(10, virObjectFreeHashData))) { + if (!(interfaces->objsName = virHashNew(virObjectFreeHashData))) { virObjectUnref(interfaces); return NULL; } diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index a2835ebf8e..3e537e512e 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -118,8 +118,7 @@ virNetworkObjNew(void) virBitmapSetBitExpand(obj->classIdMap, 2) < 0) goto error; - if (!(obj->ports = virHashCreate(10, - virNetworkObjPortFree))) + if (!(obj->ports = virHashNew(virNetworkObjPortFree))) goto error; virObjectLock(obj); @@ -351,7 +350,7 @@ virNetworkObjListNew(void) if (!(nets = virObjectRWLockableNew(virNetworkObjListClass))) return NULL; - if (!(nets->objs = virHashCreate(50, virObjectFreeHashData))) { + if (!(nets->objs = virHashNew(virObjectFreeHashData))) { virObjectUnref(nets); return NULL; } diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 9af80b8036..dcac4c36bb 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -454,7 +454,7 @@ virNodeDeviceObjListNew(void) if (!(devs = virObjectRWLockableNew(virNodeDeviceObjListClass))) return NULL; - if (!(devs->objs = virHashCreate(50, virObjectFreeHashData))) { + if (!(devs->objs = virHashNew(virObjectFreeHashData))) { virObjectUnref(devs); return NULL; } diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c index 328b1b8482..6f4ad0bae6 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -66,7 +66,7 @@ virNWFilterBindingObjListNew(void) if (!(bindings = virObjectRWLockableNew(virNWFilterBindingObjListClass))) return NULL; - if (!(bindings->objs = virHashCreate(50, virObjectFreeHashData))) { + if (!(bindings->objs = virHashNew(virObjectFreeHashData))) { virObjectUnref(bindings); return NULL; } diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index d74deb9316..a3ae64ec53 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -119,7 +119,7 @@ virSecretObjListNew(void) if (!(secrets = virObjectRWLockableNew(virSecretObjListClass))) return NULL; - if (!(secrets->objs = virHashCreate(50, virObjectFreeHashData))) { + if (!(secrets->objs = virHashNew(virObjectFreeHashData))) { virObjectUnref(secrets); return NULL; } diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 1c7d615a7a..1edcc3e074 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -172,9 +172,9 @@ virStorageVolObjListNew(void) if (!(vols = virObjectRWLockableNew(virStorageVolObjListClass))) return NULL; - if (!(vols->objsKey = virHashCreate(10, virObjectFreeHashData)) || - !(vols->objsName = virHashCreate(10, virObjectFreeHashData)) || - !(vols->objsPath = virHashCreate(10, virObjectFreeHashData))) { + if (!(vols->objsKey = virHashNew(virObjectFreeHashData)) || + !(vols->objsName = virHashNew(virObjectFreeHashData)) || + !(vols->objsPath = virHashNew(virObjectFreeHashData))) { virObjectUnref(vols); return NULL; } @@ -404,8 +404,8 @@ virStoragePoolObjListNew(void) if (!(pools = virObjectRWLockableNew(virStoragePoolObjListClass))) return NULL; - if (!(pools->objs = virHashCreate(20, virObjectFreeHashData)) || - !(pools->objsName = virHashCreate(20, virObjectFreeHashData))) { + if (!(pools->objs = virHashNew(virObjectFreeHashData)) || + !(pools->objsName = virHashNew(virObjectFreeHashData))) { virObjectUnref(pools); return NULL; } diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 29447b8f0f..f2fb7faeb0 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -332,7 +332,7 @@ hypervCreateEmbeddedParam(hypervPrivate *priv, hypervWmiClassInfoListPtr info) for (count = 0; typeinfo[count].name != NULL; count++) ; - table = virHashCreate(count, NULL); + table = virHashNew(NULL); if (table == NULL) return NULL; diff --git a/src/hypervisor/virclosecallbacks.c b/src/hypervisor/virclosecallbacks.c index 403af047fb..176dd5c263 100644 --- a/src/hypervisor/virclosecallbacks.c +++ b/src/hypervisor/virclosecallbacks.c @@ -69,7 +69,7 @@ virCloseCallbacksNew(void) if (!(closeCallbacks = virObjectLockableNew(virCloseCallbacksClass))) return NULL; - closeCallbacks->list = virHashCreate(5, virHashValueFree); + closeCallbacks->list = virHashNew(virHashValueFree); if (!closeCallbacks->list) { virObjectUnref(closeCallbacks); return NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4f9e7f6048..36e2c66d93 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2199,7 +2199,6 @@ virHashAddEntry; virHashAtomicNew; virHashAtomicSteal; virHashAtomicUpdate; -virHashCreate; virHashEqual; virHashForEach; virHashFree; diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c index 379d7a8d00..d0ce899fe8 100644 --- a/src/libxl/libxl_logger.c +++ b/src/libxl/libxl_logger.c @@ -153,7 +153,7 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel) } logger.logDir = logDir; - if ((logger.files = virHashCreate(3, libxlLoggerFileFree)) == NULL) + if ((logger.files = virHashNew(libxlLoggerFileFree)) == NULL) return NULL; path = g_strdup_printf("%s/libxl-driver.log", logDir); diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 88fa7b7957..f1dabe56cd 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -56,8 +56,6 @@ VIR_LOG_INIT("locking.lock_daemon"); -#define VIR_LOCK_DAEMON_NUM_LOCKSPACES 3 - struct _virLockDaemon { GMutex lock; virNetDaemonPtr dmn; @@ -155,8 +153,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) virObjectUnref(srv); srv = NULL; - if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, - virLockDaemonLockSpaceDataFree))) + if (!(lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree))) goto error; if (!(lockd->defaultLockspace = virLockSpaceNew(NULL))) @@ -215,8 +212,7 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) g_mutex_init(&lockd->lock); - if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, - virLockDaemonLockSpaceDataFree))) + if (!(lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree))) goto error; if (!(child = virJSONValueObjectGet(object, "defaultLockspace"))) { diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d0992b782c..a9cbaccdad 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -2005,10 +2005,10 @@ virNWFilterDHCPSnoopInit(void) virMutexInit(&virNWFilterSnoopState.activeLock) < 0) return -1; - virNWFilterSnoopState.ifnameToKey = virHashCreate(0, NULL); - virNWFilterSnoopState.active = virHashCreate(0, NULL); + virNWFilterSnoopState.ifnameToKey = virHashNew(NULL); + virNWFilterSnoopState.active = virHashNew(NULL); virNWFilterSnoopState.snoopReqs = - virHashCreate(0, virNWFilterSnoopReqRelease); + virHashNew(virNWFilterSnoopReqRelease); if (!virNWFilterSnoopState.ifnameToKey || !virNWFilterSnoopState.snoopReqs || diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index e73aa82eb4..da6f88f135 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3323,8 +3323,8 @@ ebiptablesApplyNewRules(const char *ifname, { size_t i, j; g_autoptr(virFirewall) fw = virFirewallNew(); - g_autoptr(virHashTable) chains_in_set = virHashCreate(10, NULL); - g_autoptr(virHashTable) chains_out_set = virHashCreate(10, NULL); + g_autoptr(virHashTable) chains_in_set = virHashNew(NULL); + g_autoptr(virHashTable) chains_out_set = virHashNew(NULL); bool haveEbtables = false; bool haveIptables = false; bool haveIp6tables = false; diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 99d2c0fce4..073c91550a 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -1008,7 +1008,7 @@ virNWFilterBuildAll(virNWFilterDriverStatePtr driver, VIR_DEBUG("Build all filters newFilters=%d", newFilters); if (newFilters) { - if (!(data.skipInterfaces = virHashCreate(0, NULL))) + if (!(data.skipInterfaces = virHashNew(NULL))) return -1; data.step = STEP_APPLY_NEW; diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index faf66ac84a..82797d9a64 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -777,11 +777,11 @@ virNWFilterLearnInit(void) VIR_DEBUG("Initializing IP address learning"); threadsTerminate = false; - pendingLearnReq = virHashCreate(0, freeLearnReqEntry); + pendingLearnReq = virHashNew(freeLearnReqEntry); if (!pendingLearnReq) return -1; - ifaceLockMap = virHashCreate(0, virHashValueFree); + ifaceLockMap = virHashNew(virHashValueFree); if (!ifaceLockMap) { virNWFilterLearnShutdown(); return -1; diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index f1cd12a950..0c2710a7c6 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -234,7 +234,7 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, memset(&data, 0, sizeof(data)); - if (!(namednodestable = virHashCreate(50, virJSONValueHashFree))) + if (!(namednodestable = virHashNew(virJSONValueHashFree))) return NULL; if (virJSONValueArrayForeachSteal(namednodes, @@ -242,7 +242,7 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, namednodestable) < 0) return NULL; - if (!(disks = virHashCreate(50, qemuBlockNodeNameBackingChainDataHashEntryFree))) + if (!(disks = virHashNew(qemuBlockNodeNameBackingChainDataHashEntryFree))) return NULL; data.nodenamestable = namednodestable; @@ -370,7 +370,7 @@ qemuBlockGetNodeData(virJSONValuePtr data) { g_autoptr(virHashTable) nodedata = NULL; - if (!(nodedata = virHashCreate(50, virJSONValueHashFree))) + if (!(nodedata = virHashNew(virJSONValueHashFree))) return NULL; if (virJSONValueArrayForeachSteal(data, diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9435333a0e..2249d035fb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1784,7 +1784,7 @@ virQEMUDomainCapsCacheNew(void) if (!(cache = virObjectLockableNew(virQEMUDomainCapsCacheClass))) return NULL; - if (!(cache->cache = virHashCreate(5, virObjectFreeHashData))) + if (!(cache->cache = virHashNew(virObjectFreeHashData))) return NULL; return g_steal_pointer(&cache); @@ -3114,7 +3114,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, qemuMonitorCPUPropertyPtr nmProp; size_t i; - if (!(hash = virHashCreate(0, NULL))) + if (!(hash = virHashNew(NULL))) goto cleanup; for (i = 0; i < modelInfo->nprops; i++) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index faf892fba3..f1191c1210 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -787,7 +787,7 @@ qemuStateInitialize(bool privileged, if (!(qemu_driver->hostdevMgr = virHostdevManagerGetDefault())) goto error; - if (!(qemu_driver->sharedDevices = virHashCreate(30, qemuSharedDeviceEntryFree))) + if (!(qemu_driver->sharedDevices = virHashNew(qemuSharedDeviceEntryFree))) goto error; if (qemuMigrationDstErrorInit(qemu_driver) < 0) diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index 975cbf0afd..080674ce2d 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -125,7 +125,7 @@ qemuInteropFetchConfigs(const char *name, homeConfig = g_strdup_printf("%s/qemu/%s", xdgConfig, name); } - if (!(files = virHashCreate(10, virHashValueFree))) + if (!(files = virHashNew(virHashValueFree))) return -1; if (qemuBuildFileList(files, sysLocation) < 0) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b66d278ee5..51de72b5bf 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2086,7 +2086,7 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon) QEMU_CHECK_MONITOR_NULL(mon); - if (!(table = virHashCreate(32, qemuDomainDiskInfoFree))) + if (!(table = virHashNew(qemuDomainDiskInfoFree))) return NULL; ret = qemuMonitorJSONGetBlockInfo(mon, table); @@ -2137,7 +2137,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); - if (!(*ret_stats = virHashCreate(10, virHashValueFree))) + if (!(*ret_stats = virHashNew(virHashValueFree))) goto error; ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, @@ -2878,7 +2878,7 @@ qemuMonitorGetChardevInfo(qemuMonitorPtr mon, QEMU_CHECK_MONITOR_GOTO(mon, error); - if (!(info = virHashCreate(10, qemuMonitorChardevInfoFree))) + if (!(info = virHashNew(qemuMonitorChardevInfoFree))) goto error; ret = qemuMonitorJSONGetChardevInfo(mon, info); @@ -4289,7 +4289,7 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); - if (!(*info = virHashCreate(10, virHashValueFree))) + if (!(*info = virHashNew(virHashValueFree))) return -1; if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *info)) < 0) { @@ -4593,7 +4593,7 @@ qemuMonitorGetPRManagerInfo(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); - if (!(info = virHashCreate(10, virHashValueFree))) + if (!(info = virHashNew(virHashValueFree))) goto cleanup; if (qemuMonitorJSONGetPRManagerInfo(mon, info) < 0) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 6c763ecc12..91cc0c9046 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5126,7 +5126,7 @@ qemuMonitorJSONGetAllBlockJobInfo(qemuMonitorPtr mon, } nr_results = virJSONValueArraySize(data); - if (!(blockJobs = virHashCreate(nr_results, virHashValueFree))) + if (!(blockJobs = virHashNew(virHashValueFree))) goto cleanup; for (i = 0; i < nr_results; i++) { diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c index 43f367f2a2..5e5fff3019 100644 --- a/src/qemu/qemu_qapi.c +++ b/src/qemu/qemu_qapi.c @@ -517,7 +517,7 @@ virQEMUQAPISchemaConvert(virJSONValuePtr schemareply) g_autoptr(virHashTable) schema = NULL; g_autoptr(virJSONValue) schemajson = schemareply; - if (!(schema = virHashCreate(512, virJSONValueHashFree))) + if (!(schema = virHashNew(virJSONValueHashFree))) return NULL; if (virJSONValueArrayForeachSteal(schemajson, diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 37a5662e04..3e2af53e82 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -140,7 +140,7 @@ virNetDaemonNew(void) if (!(dmn = virObjectLockableNew(virNetDaemonClass))) return NULL; - if (!(dmn->servers = virHashCreate(5, virObjectFreeHashData))) + if (!(dmn->servers = virHashNew(virObjectFreeHashData))) goto error; #ifndef WIN32 diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e40d670e97..733bcf23d9 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -685,7 +685,7 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) goto error; } - if (!(data->mcs = virHashCreate(10, NULL))) + if (!(data->mcs = virHashNew(NULL))) goto error; return 0; @@ -757,7 +757,7 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr) VIR_DEBUG("Loaded file context '%s', content context '%s'", data->file_context, data->content_context); - if (!(data->mcs = virHashCreate(10, NULL))) + if (!(data->mcs = virHashNew(NULL))) goto error; return 0; diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c index 195587e6bd..6d75e2e4fb 100644 --- a/src/util/virfilecache.c +++ b/src/util/virfilecache.c @@ -242,7 +242,7 @@ virFileCacheNew(const char *dir, if (!(cache = virObjectNew(virFileCacheClass))) return NULL; - if (!(cache->table = virHashCreate(10, virObjectFreeHashData))) + if (!(cache->table = virHashNew(virObjectFreeHashData))) goto cleanup; cache->dir = g_strdup(dir); diff --git a/src/util/virhash.c b/src/util/virhash.c index 23e12e0255..ce09f8f248 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -190,27 +190,6 @@ virHashNew(virHashDataFree dataFree) } -/** - * virHashCreate: - * @size: the size of the hash table - * @dataFree: callback to free data - * - * Create a new virHashTablePtr. - * - * Returns the newly created object. - */ -virHashTablePtr virHashCreate(ssize_t size, virHashDataFree dataFree) -{ - return virHashCreateFull(size, - dataFree, - virHashStrCode, - virHashStrEqual, - virHashStrCopy, - virHashStrPrintHuman, - virHashStrFree); -} - - virHashAtomicPtr virHashAtomicNew(virHashDataFree dataFree) { diff --git a/src/util/virhash.h b/src/util/virhash.h index baf92996a3..b136455f1f 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -112,8 +112,6 @@ typedef void (*virHashKeyFree)(void *name); * Constructor and destructor. */ virHashTablePtr virHashNew(virHashDataFree dataFree); -virHashTablePtr virHashCreate(ssize_t size, - virHashDataFree dataFree); virHashAtomicPtr virHashAtomicNew(virHashDataFree dataFree); virHashTablePtr virHashCreateFull(ssize_t size, virHashDataFree dataFree, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index b5dd2edbd3..beab42be26 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -76,9 +76,9 @@ iptablesPrivateChainCreate(virFirewallPtr fw, int ret = -1; size_t i; - if (!(chains = virHashCreate(50, NULL))) + if (!(chains = virHashNew(NULL))) goto cleanup; - if (!(links = virHashCreate(50, NULL))) + if (!(links = virHashNew(NULL))) goto cleanup; tmp = lines; diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index ef370936a3..30d6a19d1d 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -251,8 +251,7 @@ virLockSpacePtr virLockSpaceNew(const char *directory) lockspace->dir = g_strdup(directory); - if (!(lockspace->resources = virHashCreate(VIR_LOCKSPACE_TABLE_SIZE, - virLockSpaceResourceDataFree))) + if (!(lockspace->resources = virHashNew(virLockSpaceResourceDataFree))) goto error; if (directory) { @@ -299,8 +298,7 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object) return NULL; } - if (!(lockspace->resources = virHashCreate(VIR_LOCKSPACE_TABLE_SIZE, - virLockSpaceResourceDataFree))) + if (!(lockspace->resources = virHashNew(virLockSpaceResourceDataFree))) goto error; if (virJSONValueObjectHasKey(object, "directory")) { diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index 4b5e24718d..94e73f3530 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -299,7 +299,7 @@ virMacMapNew(const char *file) return NULL; virObjectLock(mgr); - if (!(mgr->macs = virHashCreate(VIR_MAC_HASH_TABLE_SIZE, NULL))) + if (!(mgr->macs = virHashNew(NULL))) goto error; if (file && diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 82388ae544..33db43cd92 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4310,7 +4310,7 @@ virStorageFileCanonicalizePath(const char *path, beginDoubleSlash = true; } - if (!(cycle = virHashCreate(10, NULL))) + if (!(cycle = virHashNew(NULL))) goto cleanup; if (!(components = virStringSplitCount(path, "/", 0, &ncomponents))) @@ -5317,7 +5317,7 @@ virStorageFileGetMetadata(virStorageSourcePtr src, src->path, src->format, (unsigned int)uid, (unsigned int)gid, report_broken); - if (!(cycle = virHashCreate(5, NULL))) + if (!(cycle = virHashNew(NULL))) return -1; if (src->format <= VIR_STORAGE_FILE_NONE) { diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8373ee6509..09f80a1619 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -896,7 +896,7 @@ virSystemdActivationNew(virSystemdActivationMap *map, VIR_DEBUG("Activated with %d FDs", nfds); act = g_new0(virSystemdActivation, 1); - if (!(act->fds = virHashCreate(10, virSystemdActivationEntryFree))) + if (!(act->fds = virHashNew(virSystemdActivationEntryFree))) goto error; fdnames = getenv("LISTEN_FDNAMES"); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0611fdfd34..f50ecdeb3f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1649,8 +1649,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(blockDevices = virHashCreate(32, virHashValueFree)) || - !(expectedBlockDevices = virHashCreate(32, virHashValueFree))) + if (!(blockDevices = virHashNew(virHashValueFree)) || + !(expectedBlockDevices = virHashNew(virHashValueFree))) goto cleanup; info = g_new0(struct qemuDomainDiskInfo, 1); @@ -1811,7 +1811,7 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(blockstats = virHashCreate(10, virHashValueFree))) + if (!(blockstats = virHashNew(virHashValueFree))) goto cleanup; if (qemuMonitorTestAddItem(test, "query-blockstats", reply) < 0) @@ -2018,8 +2018,8 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(info = virHashCreate(32, qemuMonitorChardevInfoFree)) || - !(expectedInfo = virHashCreate(32, NULL))) + if (!(info = virHashNew(qemuMonitorChardevInfoFree)) || + !(expectedInfo = virHashNew(NULL))) goto cleanup; if (virHashAddEntry(expectedInfo, "charserial1", &info1) < 0 || diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index 6366432dd4..c1344b3daa 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -85,12 +85,12 @@ init_hash(void) if (xattr_paths) return; - if (!(xattr_paths = virHashCreate(10, virHashValueFree))) { + if (!(xattr_paths = virHashNew(virHashValueFree))) { fprintf(stderr, "Unable to create hash table for XATTR paths\n"); abort(); } - if (!(chown_paths = virHashCreate(10, virHashValueFree))) { + if (!(chown_paths = virHashNew(virHashValueFree))) { fprintf(stderr, "Unable to create hash table for chowned paths\n"); abort(); } diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 5b23888a98..3f50d46d87 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -576,7 +576,7 @@ testQemuGetLatestCaps(void) virHashTablePtr capslatest; size_t i; - if (!(capslatest = virHashCreate(4, virHashValueFree))) + if (!(capslatest = virHashNew(virHashValueFree))) goto error; VIR_TEST_VERBOSE(""); diff --git a/tests/virhashtest.c b/tests/virhashtest.c index bc93c07d1f..0ba5b9ed31 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -388,7 +388,7 @@ testHashGetItems(const void *data G_GNUC_UNUSED) char value2[] = "2"; char value3[] = "3"; - if (!(hash = virHashCreate(0, NULL)) || + if (!(hash = virHashNew(NULL)) || virHashAddEntry(hash, keya, value3) < 0 || virHashAddEntry(hash, keyc, value1) < 0 || virHashAddEntry(hash, keyb, value2) < 0) { @@ -458,8 +458,8 @@ testHashEqual(const void *data G_GNUC_UNUSED) char value3_u[] = "O"; char value4_u[] = "P"; - if (!(hash1 = virHashCreate(0, NULL)) || - !(hash2 = virHashCreate(0, NULL)) || + if (!(hash1 = virHashNew(NULL)) || + !(hash2 = virHashNew(NULL)) || virHashAddEntry(hash1, keya, value1_l) < 0 || virHashAddEntry(hash1, keyb, value2_l) < 0 || virHashAddEntry(hash1, keyc, value3_l) < 0 || @@ -508,7 +508,7 @@ testHashDuplicate(const void *data G_GNUC_UNUSED) { g_autoptr(virHashTable) hash = NULL; - if (!(hash = virHashCreate(0, NULL))) + if (!(hash = virHashNew(NULL))) return -1; if (virHashAddEntry(hash, "a", NULL) < 0) { -- 2.26.2

On Thu, Oct 22, 2020 at 11:35:01AM +0200, Peter Krempa wrote:
It doesn't make much sense to configure the bucket count in the hash table for each case specifically. Replace all calls of virHashCreate with virHashNew which has a pre-set size and remove virHashCreate completely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 4 ++-- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 4 ++-- src/conf/virinterfaceobj.c | 2 +- src/conf/virnetworkobj.c | 5 ++--- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingobjlist.c | 2 +- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 10 +++++----- src/hyperv/hyperv_wmi.c | 2 +- src/hypervisor/virclosecallbacks.c | 2 +- src/libvirt_private.syms | 1 - src/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 8 ++------ src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++--- src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++-- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 4 ++-- src/qemu/qemu_block.c | 6 +++--- src/qemu/qemu_capabilities.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_interop_config.c | 2 +- src/qemu/qemu_monitor.c | 10 +++++----- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_qapi.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/security/security_selinux.c | 4 ++-- src/util/virfilecache.c | 2 +- src/util/virhash.c | 21 --------------------- src/util/virhash.h | 2 -- src/util/viriptables.c | 4 ++-- src/util/virlockspace.c | 6 ++---- src/util/virmacmap.c | 2 +- src/util/virstoragefile.c | 4 ++-- src/util/virsystemd.c | 2 +- tests/qemumonitorjsontest.c | 10 +++++----- tests/qemusecuritymock.c | 4 ++-- tests/testutilsqemu.c | 2 +- tests/virhashtest.c | 8 ++++---- 41 files changed, 69 insertions(+), 100 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Use 'g_free' directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/hypervisor/virclosecallbacks.c | 2 +- src/libvirt_private.syms | 1 - src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/qemu/qemu_interop_config.c | 2 +- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_monitor.c | 6 +++--- src/qemu/qemu_monitor_json.c | 2 +- src/util/virhash.c | 7 ------- src/util/virhash.h | 3 --- tests/qemumonitorjsontest.c | 6 +++--- tests/qemusecuritymock.c | 4 ++-- tests/testutilsqemu.c | 2 +- 13 files changed, 15 insertions(+), 26 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index de344186ec..37dad20ade 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1389,7 +1389,7 @@ virDomainCCWAddressSetCreate(void) addrs = g_new0(virDomainCCWAddressSet, 1); - if (!(addrs->defined = virHashNew(virHashValueFree))) + if (!(addrs->defined = virHashNew(g_free))) goto error; /* must use cssid = 0xfe (254) for virtio-ccw devices */ diff --git a/src/hypervisor/virclosecallbacks.c b/src/hypervisor/virclosecallbacks.c index 176dd5c263..a73ab818da 100644 --- a/src/hypervisor/virclosecallbacks.c +++ b/src/hypervisor/virclosecallbacks.c @@ -69,7 +69,7 @@ virCloseCallbacksNew(void) if (!(closeCallbacks = virObjectLockableNew(virCloseCallbacksClass))) return NULL; - closeCallbacks->list = virHashNew(virHashValueFree); + closeCallbacks->list = virHashNew(g_free); if (!closeCallbacks->list) { virObjectUnref(closeCallbacks); return NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 36e2c66d93..926e982e0b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2213,7 +2213,6 @@ virHashSearch; virHashSize; virHashSteal; virHashUpdateEntry; -virHashValueFree; # util/virhashcode.h diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 82797d9a64..6dc535a4fe 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -781,7 +781,7 @@ virNWFilterLearnInit(void) if (!pendingLearnReq) return -1; - ifaceLockMap = virHashNew(virHashValueFree); + ifaceLockMap = virHashNew(g_free); if (!ifaceLockMap) { virNWFilterLearnShutdown(); return -1; diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index 080674ce2d..53b251f056 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -125,7 +125,7 @@ qemuInteropFetchConfigs(const char *name, homeConfig = g_strdup_printf("%s/qemu/%s", xdgConfig, name); } - if (!(files = virHashNew(virHashValueFree))) + if (!(files = virHashNew(g_free))) return -1; if (qemuBuildFileList(files, sysLocation) < 0) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index abe797759d..708c2cced7 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -435,7 +435,7 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - g_autoptr(virHashTable) stats = virHashNew(virHashValueFree); + g_autoptr(virHashTable) stats = virHashNew(g_free); bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); size_t i; int rc; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 51de72b5bf..69d81b20c2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2137,7 +2137,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); - if (!(*ret_stats = virHashNew(virHashValueFree))) + if (!(*ret_stats = virHashNew(g_free))) goto error; ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, @@ -4289,7 +4289,7 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); - if (!(*info = virHashNew(virHashValueFree))) + if (!(*info = virHashNew(g_free))) return -1; if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *info)) < 0) { @@ -4593,7 +4593,7 @@ qemuMonitorGetPRManagerInfo(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); - if (!(info = virHashNew(virHashValueFree))) + if (!(info = virHashNew(g_free))) goto cleanup; if (qemuMonitorJSONGetPRManagerInfo(mon, info) < 0) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 91cc0c9046..2689ad50b9 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5126,7 +5126,7 @@ qemuMonitorJSONGetAllBlockJobInfo(qemuMonitorPtr mon, } nr_results = virJSONValueArraySize(data); - if (!(blockJobs = virHashNew(virHashValueFree))) + if (!(blockJobs = virHashNew(g_free))) goto cleanup; for (i = 0; i < nr_results; i++) { diff --git a/src/util/virhash.c b/src/util/virhash.c index ce09f8f248..c781e1d5a5 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -111,13 +111,6 @@ static void virHashStrFree(void *name) } -void -virHashValueFree(void *value) -{ - VIR_FREE(value); -} - - static size_t virHashComputeKey(const virHashTable *table, const void *name) { diff --git a/src/util/virhash.h b/src/util/virhash.h index b136455f1f..2d221dce25 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -202,7 +202,4 @@ ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void void *virHashSearch(const virHashTable *table, virHashSearcher iter, const void *data, void **name); -/* Convenience for when VIR_FREE(value) is sufficient as a data freer. */ -void virHashValueFree(void *value); - G_DEFINE_AUTOPTR_CLEANUP_FUNC(virHashTable, virHashFree); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index f50ecdeb3f..da7fd4625a 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1649,8 +1649,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(blockDevices = virHashNew(virHashValueFree)) || - !(expectedBlockDevices = virHashNew(virHashValueFree))) + if (!(blockDevices = virHashNew(g_free)) || + !(expectedBlockDevices = virHashNew(g_free))) goto cleanup; info = g_new0(struct qemuDomainDiskInfo, 1); @@ -1811,7 +1811,7 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(blockstats = virHashNew(virHashValueFree))) + if (!(blockstats = virHashNew(g_free))) goto cleanup; if (qemuMonitorTestAddItem(test, "query-blockstats", reply) < 0) diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index c1344b3daa..839be55665 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -85,12 +85,12 @@ init_hash(void) if (xattr_paths) return; - if (!(xattr_paths = virHashNew(virHashValueFree))) { + if (!(xattr_paths = virHashNew(g_free))) { fprintf(stderr, "Unable to create hash table for XATTR paths\n"); abort(); } - if (!(chown_paths = virHashNew(virHashValueFree))) { + if (!(chown_paths = virHashNew(g_free))) { fprintf(stderr, "Unable to create hash table for chowned paths\n"); abort(); } diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 3f50d46d87..4defba0b7b 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -576,7 +576,7 @@ testQemuGetLatestCaps(void) virHashTablePtr capslatest; size_t i; - if (!(capslatest = virHashNew(virHashValueFree))) + if (!(capslatest = virHashNew(g_free))) goto error; VIR_TEST_VERBOSE(""); -- 2.26.2

On Thu, Oct 22, 2020 at 11:35:02AM +0200, Peter Krempa wrote:
Use 'g_free' directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/hypervisor/virclosecallbacks.c | 2 +- src/libvirt_private.syms | 1 - src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/qemu/qemu_interop_config.c | 2 +- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_monitor.c | 6 +++--- src/qemu/qemu_monitor_json.c | 2 +- src/util/virhash.c | 7 ------- src/util/virhash.h | 3 --- tests/qemumonitorjsontest.c | 6 +++--- tests/qemusecuritymock.c | 4 ++-- tests/testutilsqemu.c | 2 +- 13 files changed, 15 insertions(+), 26 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

The only place we call it is in virHashNew. Move the code to virHashNew and remove virHashCreateFull. Code wishing to use non-strings as hash table keys will be better off using glib's GHashTable directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhash.c | 53 +++++++++------------------------------------- src/util/virhash.h | 7 ------ 2 files changed, 10 insertions(+), 50 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index c781e1d5a5..8d56b4bb85 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -118,43 +118,31 @@ virHashComputeKey(const virHashTable *table, const void *name) return value % table->size; } + /** - * virHashCreateFull: - * @size: the size of the hash table + * virHashNew: * @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. */ -virHashTablePtr virHashCreateFull(ssize_t size, - virHashDataFree dataFree, - virHashKeyCode keyCode, - virHashKeyEqual keyEqual, - virHashKeyCopy keyCopy, - virHashKeyPrintHuman keyPrint, - virHashKeyFree keyFree) +virHashTablePtr +virHashNew(virHashDataFree dataFree) { virHashTablePtr table = NULL; - if (size <= 0) - size = 256; - table = g_new0(virHashTable, 1); table->seed = virRandomBits(32); - table->size = size; + table->size = 32; table->nbElems = 0; table->dataFree = dataFree; - table->keyCode = keyCode; - table->keyEqual = keyEqual; - table->keyCopy = keyCopy; - table->keyPrint = keyPrint; - table->keyFree = keyFree; + table->keyCode = virHashStrCode; + table->keyEqual = virHashStrEqual; + table->keyCopy = virHashStrCopy; + table->keyPrint = virHashStrPrintHuman; + table->keyFree = virHashStrFree; table->table = g_new0(virHashEntryPtr, table->size); @@ -162,27 +150,6 @@ virHashTablePtr virHashCreateFull(ssize_t size, } -/** - * virHashNew: - * @dataFree: callback to free data - * - * Create a new virHashTablePtr. - * - * Returns the newly created object. - */ -virHashTablePtr -virHashNew(virHashDataFree dataFree) -{ - return virHashCreateFull(32, - dataFree, - virHashStrCode, - virHashStrEqual, - virHashStrCopy, - virHashStrPrintHuman, - virHashStrFree); -} - - virHashAtomicPtr virHashAtomicNew(virHashDataFree dataFree) { diff --git a/src/util/virhash.h b/src/util/virhash.h index 2d221dce25..a9b022f362 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -113,13 +113,6 @@ typedef void (*virHashKeyFree)(void *name); */ virHashTablePtr virHashNew(virHashDataFree dataFree); virHashAtomicPtr virHashAtomicNew(virHashDataFree dataFree); -virHashTablePtr virHashCreateFull(ssize_t size, - virHashDataFree dataFree, - virHashKeyCode keyCode, - virHashKeyEqual keyEqual, - virHashKeyCopy keyCopy, - virHashKeyPrintHuman keyPrint, - virHashKeyFree keyFree); void virHashFree(virHashTablePtr table); ssize_t virHashSize(const virHashTable *table); -- 2.26.2

On Thu, Oct 22, 2020 at 11:35:03AM +0200, Peter Krempa wrote:
The only place we call it is in virHashNew. Move the code to virHashNew and remove virHashCreateFull.
Code wishing to use non-strings as hash table keys will be better off using glib's GHashTable directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhash.c | 53 +++++++++------------------------------------- src/util/virhash.h | 7 ------ 2 files changed, 10 insertions(+), 50 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

All users of virHashTable pass strings as the name/key of the entry. Make this an official requirement by turning the variables to 'const char *'. For any other case it's better to use glib's GHashTable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/nwfilter_params.c | 2 +- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 6 ++-- src/conf/virdomainobjlist.c | 12 ++++---- src/conf/virinterfaceobj.c | 10 +++--- src/conf/virnetworkobj.c | 16 +++++----- src/conf/virnodedeviceobj.c | 18 +++++------ src/conf/virnwfilterbindingobjlist.c | 4 +-- src/conf/virsecretobj.c | 8 ++--- src/conf/virstorageobj.c | 22 +++++++------- src/hypervisor/virclosecallbacks.c | 2 +- src/locking/lock_daemon.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 ++-- src/nwfilter/nwfilter_gentech_driver.c | 6 ++-- src/qemu/qemu_blockjob.c | 2 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_domain.c | 6 ++-- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_snapshot.c | 4 +-- src/rpc/virnetdaemon.c | 14 ++++----- src/test/test_driver.c | 6 ++-- src/util/virfilecache.c | 2 +- src/util/virhash.c | 42 +++++++++++++------------- src/util/virhash.h | 32 ++++++++++---------- src/util/virlockspace.c | 2 +- src/util/virmacmap.c | 4 +-- tests/qemumonitorjsontest.c | 2 +- tests/qemusecuritymock.c | 8 ++--- tests/virhashtest.c | 10 +++--- 31 files changed, 129 insertions(+), 129 deletions(-) diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index fd05b45ca3..73160a38a4 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -620,7 +620,7 @@ struct addToTableStruct { static int -addToTable(void *payload, const void *name, void *data) +addToTable(void *payload, const char *name, void *data) { struct addToTableStruct *atts = (struct addToTableStruct *)data; virNWFilterVarValuePtr val; diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 0d2c9503ab..5e5c03d03b 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -279,7 +279,7 @@ virChrdevsPtr virChrdevAlloc(void) * Helper to clear stream callbacks when freeing the hash */ static int virChrdevFreeClearCallbacks(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data G_GNUC_UNUSED) { virChrdevHashEntry *ent = payload; diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 43c77e6c54..511bf1d415 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -76,7 +76,7 @@ struct moment_act_on_descendant { static int virDomainMomentActOnDescendant(void *payload, - const void *name, + const char *name, void *data) { virDomainMomentObjPtr obj = payload; @@ -307,7 +307,7 @@ struct virDomainMomentNameData { static int virDomainMomentObjListCopyNames(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virDomainMomentObjPtr obj = payload; @@ -491,7 +491,7 @@ struct moment_set_relation { }; static int virDomainMomentSetRelations(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr obj = payload; diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 9e8757eff9..e9a4b271df 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -95,7 +95,7 @@ static void virDomainObjListDispose(void *obj) static int virDomainObjListSearchID(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *data) { virDomainObjPtr obj = (virDomainObjPtr)payload; @@ -649,7 +649,7 @@ struct virDomainObjListData { static int virDomainObjListCount(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virDomainObjPtr obj = payload; @@ -696,7 +696,7 @@ struct virDomainIDData { static int virDomainObjListCopyActiveIDs(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virDomainObjPtr obj = payload; @@ -741,7 +741,7 @@ struct virDomainNameData { static int virDomainObjListCopyInactiveNames(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virDomainObjPtr obj = payload; @@ -797,7 +797,7 @@ struct virDomainListIterData { static int virDomainObjListHelper(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virDomainListIterData *data = opaque; @@ -925,7 +925,7 @@ struct virDomainListData { static int virDomainObjListCollectIterator(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virDomainListData *data = opaque; diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index 1e29e12148..faf047dc5f 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -161,7 +161,7 @@ struct _virInterfaceObjFindMACData { static int virInterfaceObjListFindByMACStringCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virInterfaceObjPtr obj = payload; @@ -269,7 +269,7 @@ struct _virInterfaceObjListExportData { static int virInterfaceObjListExportCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virInterfaceObjListExportDataPtr data = opaque; @@ -361,7 +361,7 @@ struct _virInterfaceObjListCloneData { static int virInterfaceObjListCloneCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virInterfaceObjPtr srcObj = payload; @@ -480,7 +480,7 @@ struct _virInterfaceObjNumOfInterfacesData { static int virInterfaceObjListNumOfInterfacesCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virInterfaceObjPtr obj = payload; @@ -522,7 +522,7 @@ struct _virInterfaceObjGetNamesData { static int virInterfaceObjListGetNamesCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virInterfaceObjPtr obj = payload; diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 3e537e512e..46205b163c 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -402,7 +402,7 @@ virNetworkObjFindByUUID(virNetworkObjListPtr nets, static int virNetworkObjSearchName(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *data) { virNetworkObjPtr obj = (virNetworkObjPtr) payload; @@ -1178,7 +1178,7 @@ struct virNetworkObjBridgeInUseHelperData { static int virNetworkObjBridgeInUseHelper(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { int ret; @@ -1355,7 +1355,7 @@ struct _virNetworkObjListExportData { static int virNetworkObjListExportCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virNetworkObjListExportDataPtr data = opaque; @@ -1439,7 +1439,7 @@ struct virNetworkObjListForEachHelperData { static int virNetworkObjListForEachHelper(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virNetworkObjListForEachHelperData *data = opaque; @@ -1489,7 +1489,7 @@ struct virNetworkObjListGetHelperData { static int virNetworkObjListGetHelper(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virNetworkObjListGetHelperData *data = opaque; @@ -1576,7 +1576,7 @@ struct virNetworkObjListPruneHelperData { static int virNetworkObjListPruneHelper(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { const struct virNetworkObjListPruneHelperData *data = opaque; @@ -1756,7 +1756,7 @@ struct _virNetworkObjPortListExportData { static int virNetworkObjPortListExportCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virNetworkObjPortListExportDataPtr data = opaque; @@ -1834,7 +1834,7 @@ struct _virNetworkObjPortListForEachData { static int virNetworkObjPortForEachCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virNetworkObjPortListForEachData *data = opaque; diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index dcac4c36bb..f240abf315 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -199,7 +199,7 @@ virNodeDeviceObjListSearch(virNodeDeviceObjListPtr devs, static int virNodeDeviceObjListFindBySysfsPathCallback(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virNodeDeviceObjPtr obj = (virNodeDeviceObjPtr) payload; @@ -256,7 +256,7 @@ struct virNodeDeviceObjListFindByWWNsData { static int virNodeDeviceObjListFindByWWNsCallback(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virNodeDeviceObjPtr obj = (virNodeDeviceObjPtr) payload; @@ -292,7 +292,7 @@ virNodeDeviceObjListFindByWWNs(virNodeDeviceObjListPtr devs, static int virNodeDeviceObjListFindByFabricWWNCallback(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virNodeDeviceObjPtr obj = (virNodeDeviceObjPtr) payload; @@ -322,7 +322,7 @@ virNodeDeviceObjListFindByFabricWWN(virNodeDeviceObjListPtr devs, static int virNodeDeviceObjListFindByCapCallback(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virNodeDeviceObjPtr obj = (virNodeDeviceObjPtr) payload; @@ -354,7 +354,7 @@ struct virNodeDeviceObjListFindSCSIHostByWWNsData { static int virNodeDeviceObjListFindSCSIHostByWWNsCallback(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virNodeDeviceObjPtr obj = (virNodeDeviceObjPtr) payload; @@ -401,7 +401,7 @@ virNodeDeviceObjListFindSCSIHostByWWNs(virNodeDeviceObjListPtr devs, static int virNodeDeviceObjListFindMediatedDeviceByUUIDCallback(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virNodeDeviceObjPtr obj = (virNodeDeviceObjPtr) payload; @@ -729,7 +729,7 @@ struct virNodeDeviceCountData { static int virNodeDeviceObjListNumOfDevicesCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virNodeDeviceObjPtr obj = payload; @@ -777,7 +777,7 @@ struct virNodeDeviceGetNamesData { static int virNodeDeviceObjListGetNamesCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virNodeDeviceObjPtr obj = payload; @@ -884,7 +884,7 @@ struct _virNodeDeviceObjListExportData { static int virNodeDeviceObjListExportCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virNodeDeviceObjPtr obj = payload; diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c index 6f4ad0bae6..4cbb62abfa 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -345,7 +345,7 @@ struct virNWFilterBindingListIterData { static int virNWFilterBindingObjListHelper(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virNWFilterBindingListIterData *data = opaque; @@ -379,7 +379,7 @@ struct virNWFilterBindingListData { static int virNWFilterBindingObjListCollectIterator(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virNWFilterBindingListData *data = opaque; diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index a3ae64ec53..146210fbe7 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -197,7 +197,7 @@ virSecretObjListFindByUUID(virSecretObjListPtr secrets, static int virSecretObjSearchName(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virSecretObjPtr obj = (virSecretObjPtr) payload; @@ -410,7 +410,7 @@ struct virSecretCountData { static int virSecretObjListNumOfSecretsCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virSecretCountData *data = opaque; @@ -443,7 +443,7 @@ struct virSecretListData { static int virSecretObjListGetUUIDsCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct virSecretListData *data = opaque; @@ -534,7 +534,7 @@ struct _virSecretObjListExportData { static int virSecretObjListExportCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virSecretObjListExportDataPtr data = opaque; diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 1edcc3e074..219594582c 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -421,7 +421,7 @@ struct _virStoragePoolObjListForEachData { static int virStoragePoolObjListForEachCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virStoragePoolObjPtr obj = payload; @@ -477,7 +477,7 @@ struct _virStoragePoolObjListSearchData { static int virStoragePoolObjListSearchCb(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virStoragePoolObjPtr obj = (virStoragePoolObjPtr) payload; @@ -728,7 +728,7 @@ struct _virStoragePoolObjForEachVolData { static int virStoragePoolObjForEachVolumeCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { int ret = 0; @@ -767,7 +767,7 @@ struct _virStoragePoolObjSearchVolData { static int virStoragePoolObjSearchVolumeCb(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virStorageVolObjPtr volobj = (virStorageVolObjPtr) payload; @@ -864,7 +864,7 @@ struct _virStorageVolObjCountData { static int virStoragePoolObjNumOfVolumesCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virStorageVolObjPtr volobj = payload; @@ -913,7 +913,7 @@ struct _virStorageVolObjNameData { static int virStoragePoolObjVolumeGetNamesCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virStorageVolObjPtr volobj = payload; @@ -983,7 +983,7 @@ struct _virStoragePoolObjVolumeListExportData { static int virStoragePoolObjVolumeListExportCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virStorageVolObjPtr volobj = payload; @@ -1430,7 +1430,7 @@ struct _virStoragePoolObjFindDuplicateData { static int virStoragePoolObjSourceFindDuplicateCb(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virStoragePoolObjPtr obj = (virStoragePoolObjPtr) payload; @@ -1832,7 +1832,7 @@ struct _virStoragePoolCountData { static int virStoragePoolObjNumOfStoragePoolsCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virStoragePoolObjPtr obj = payload; @@ -1884,7 +1884,7 @@ struct _virStoragePoolNameData { static int virStoragePoolObjGetNamesCb(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virStoragePoolObjPtr obj = payload; @@ -2027,7 +2027,7 @@ struct _virStoragePoolObjListExportData { static int virStoragePoolObjListExportCallback(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virStoragePoolObjPtr obj = payload; diff --git a/src/hypervisor/virclosecallbacks.c b/src/hypervisor/virclosecallbacks.c index a73ab818da..d87fe84505 100644 --- a/src/hypervisor/virclosecallbacks.c +++ b/src/hypervisor/virclosecallbacks.c @@ -246,7 +246,7 @@ struct virCloseCallbacksData { static int virCloseCallbacksGetOne(void *payload, - const void *key, + const char *key, void *opaque) { struct virCloseCallbacksData *data = opaque; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index f1dabe56cd..8f16dfd064 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -362,7 +362,7 @@ struct virLockDaemonClientReleaseData { static int virLockDaemonClientReleaseLockspace(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virLockSpacePtr lockspace = payload; diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index a9cbaccdad..c425af497c 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1754,7 +1754,7 @@ virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLeasePtr ipl) */ static int virNWFilterSnoopPruneIter(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *data G_GNUC_UNUSED) { virNWFilterSnoopReqPtr req = (virNWFilterSnoopReqPtr)payload; @@ -1784,7 +1784,7 @@ virNWFilterSnoopPruneIter(const void *payload, */ static int virNWFilterSnoopSaveIter(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virNWFilterSnoopReqPtr req = payload; @@ -1951,7 +1951,7 @@ virNWFilterSnoopJoinThreads(void) */ static int virNWFilterSnoopRemAllReqIter(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *data G_GNUC_UNUSED) { virNWFilterSnoopReqPtr req = (virNWFilterSnoopReqPtr)payload; diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 073c91550a..ca6455c70d 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -140,12 +140,12 @@ struct printString static int -printString(void *payload G_GNUC_UNUSED, const void *name, void *data) +printString(void *payload G_GNUC_UNUSED, const char *name, void *data) { struct printString *ps = data; - if ((STREQ((char *)name, NWFILTER_STD_VAR_IP) && !ps->reportIP) || - (STREQ((char *)name, NWFILTER_STD_VAR_MAC) && !ps->reportMAC)) + if ((STREQ(name, NWFILTER_STD_VAR_IP) && !ps->reportIP) || + (STREQ(name, NWFILTER_STD_VAR_MAC) && !ps->reportMAC)) return 0; if (virBufferUse(&ps->buf) && ps->separator) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index c49c98e547..ed4a32e964 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -481,7 +481,7 @@ qemuBlockJobIsRunning(qemuBlockJobDataPtr job) /* returns 1 for a job we didn't reconnect to */ static int qemuBlockJobRefreshJobsFindInactive(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *data G_GNUC_UNUSED) { const qemuBlockJobData *job = payload; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2249d035fb..a67fb785b5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2156,7 +2156,7 @@ struct virQEMUCapsSearchDomcapsData { static int virQEMUCapsSearchDomcaps(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virDomainCapsPtr domCaps = (virDomainCapsPtr) payload; diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index f45ab29d4c..fb76c211f8 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -744,7 +744,7 @@ struct virQEMUCheckpointReparent { static int qemuCheckpointReparentChildren(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr moment = payload; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bea43a1aba..161b369712 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2106,7 +2106,7 @@ qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobDataPtr job, static int qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { struct qemuDomainPrivateBlockJobFormatData *data = opaque; @@ -6641,7 +6641,7 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, /* Hash iterator callback to discard multiple snapshots. */ int qemuDomainMomentDiscardAll(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr moment = payload; @@ -10718,7 +10718,7 @@ qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason) static int qemuDomainDefHasManagedPRBlockjobIterator(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { qemuBlockJobDataPtr job = payload; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 91b3b67cb6..fc69678f9b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -638,7 +638,7 @@ struct _virQEMUMomentRemove { }; int qemuDomainMomentDiscardAll(void *payload, - const void *name, + const char *name, void *data); int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 89f6fd1499..91db2319fb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8067,7 +8067,7 @@ qemuProcessRefreshCPU(virQEMUDriverPtr driver, static int qemuProcessRefreshLegacyBlockjob(void *payload, - const void *name, + const char *name, void *opaque) { const char *jobname = name; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 47df102817..a6241ab5d4 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -75,7 +75,7 @@ qemuSnapObjFromSnapshot(virDomainObjPtr vm, /* Count how many snapshots in a set are external snapshots. */ static int qemuSnapshotCountExternal(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr snap = payload; @@ -2265,7 +2265,7 @@ struct _virQEMUMomentReparent { static int qemuSnapshotChildrenReparent(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr moment = payload; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 3e2af53e82..ce13f0d927 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -86,7 +86,7 @@ static virClassPtr virNetDaemonClass; static int daemonServerClose(void *payload, - const void *key G_GNUC_UNUSED, + const char *key G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED); static void @@ -228,7 +228,7 @@ struct collectData { static int collectServers(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque) { virNetServerPtr srv = virObjectRef(payload); @@ -731,7 +731,7 @@ virNetDaemonAutoShutdownTimer(int timerid G_GNUC_UNUSED, static int daemonServerUpdateServices(void *payload, - const void *key G_GNUC_UNUSED, + const char *key G_GNUC_UNUSED, void *opaque) { bool *enable = opaque; @@ -752,7 +752,7 @@ virNetDaemonUpdateServices(virNetDaemonPtr dmn, static int daemonServerProcessClients(void *payload, - const void *key G_GNUC_UNUSED, + const char *key G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { virNetServerPtr srv = payload; @@ -763,7 +763,7 @@ daemonServerProcessClients(void *payload, static int daemonServerShutdownWait(void *payload, - const void *key G_GNUC_UNUSED, + const char *key G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { virNetServerPtr srv = payload; @@ -913,7 +913,7 @@ virNetDaemonQuit(virNetDaemonPtr dmn) static int daemonServerClose(void *payload, - const void *key G_GNUC_UNUSED, + const char *key G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { virNetServerPtr srv = payload; @@ -924,7 +924,7 @@ daemonServerClose(void *payload, static int daemonServerHasClients(void *payload, - const void *key G_GNUC_UNUSED, + const char *key G_GNUC_UNUSED, void *opaque) { bool *clients = opaque; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index bb26fc247c..5c02a8ebb0 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -8592,7 +8592,7 @@ struct _testMomentRemoveData { static int testDomainSnapshotDiscardAll(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr snap = payload; @@ -8612,7 +8612,7 @@ struct _testMomentReparentData { static int testDomainMomentReparentChildren(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr moment = payload; @@ -8906,7 +8906,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, static int testDomainCheckpointDiscardAll(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data) { virDomainMomentObjPtr chk = payload; diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c index 6d75e2e4fb..cfb1d1d247 100644 --- a/src/util/virfilecache.c +++ b/src/util/virfilecache.c @@ -337,7 +337,7 @@ virFileCacheLookupByFunc(virFileCachePtr cache, virObjectLock(cache); - data = virHashSearch(cache->table, iter, iterData, (void **)&name); + data = virHashSearch(cache->table, iter, iterData, &name); virFileCacheValidate(cache, name, &data); virObjectRef(data); diff --git a/src/util/virhash.c b/src/util/virhash.c index 8d56b4bb85..f386839d6b 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -43,7 +43,7 @@ typedef struct _virHashEntry virHashEntry; typedef virHashEntry *virHashEntryPtr; struct _virHashEntry { struct _virHashEntry *next; - void *name; + char *name; void *payload; }; @@ -82,37 +82,37 @@ static int virHashAtomicOnceInit(void) VIR_ONCE_GLOBAL_INIT(virHashAtomic); -static uint32_t virHashStrCode(const void *name, uint32_t seed) +static uint32_t virHashStrCode(const char *name, uint32_t seed) { return virHashCodeGen(name, strlen(name), seed); } -static bool virHashStrEqual(const void *namea, const void *nameb) +static bool virHashStrEqual(const char *namea, const char *nameb) { return STREQ(namea, nameb); } -static void *virHashStrCopy(const void *name) +static char *virHashStrCopy(const char *name) { return g_strdup(name); } static char * -virHashStrPrintHuman(const void *name) +virHashStrPrintHuman(const char *name) { return g_strdup(name); } -static void virHashStrFree(void *name) +static void virHashStrFree(char *name) { VIR_FREE(name); } static size_t -virHashComputeKey(const virHashTable *table, const void *name) +virHashComputeKey(const virHashTable *table, const char *name) { uint32_t value = table->keyCode(name, table->seed); return value % table->size; @@ -272,7 +272,7 @@ virHashFree(virHashTablePtr table) } static int -virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, +virHashAddOrUpdateEntry(virHashTablePtr table, const char *name, void *userdata, bool is_update) { @@ -337,7 +337,7 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, * Returns 0 the addition succeeded and -1 in case of error. */ int -virHashAddEntry(virHashTablePtr table, const void *name, void *userdata) +virHashAddEntry(virHashTablePtr table, const char *name, void *userdata) { return virHashAddOrUpdateEntry(table, name, userdata, false); } @@ -355,7 +355,7 @@ virHashAddEntry(virHashTablePtr table, const void *name, void *userdata) * Returns 0 the addition succeeded and -1 in case of error. */ int -virHashUpdateEntry(virHashTablePtr table, const void *name, +virHashUpdateEntry(virHashTablePtr table, const char *name, void *userdata) { return virHashAddOrUpdateEntry(table, name, userdata, true); @@ -363,7 +363,7 @@ virHashUpdateEntry(virHashTablePtr table, const void *name, int virHashAtomicUpdate(virHashAtomicPtr table, - const void *name, + const char *name, void *userdata) { int ret; @@ -378,7 +378,7 @@ virHashAtomicUpdate(virHashAtomicPtr table, static virHashEntryPtr virHashGetEntry(const virHashTable *table, - const void *name) + const char *name) { size_t key; virHashEntryPtr entry; @@ -406,7 +406,7 @@ virHashGetEntry(const virHashTable *table, * Returns a pointer to the userdata */ void * -virHashLookup(const virHashTable *table, const void *name) +virHashLookup(const virHashTable *table, const char *name) { virHashEntryPtr entry = virHashGetEntry(table, name); @@ -428,7 +428,7 @@ virHashLookup(const virHashTable *table, const void *name) */ bool virHashHasEntry(const virHashTable *table, - const void *name) + const char *name) { return !!virHashGetEntry(table, name); } @@ -444,7 +444,7 @@ virHashHasEntry(const virHashTable *table, * * Returns a pointer to the userdata */ -void *virHashSteal(virHashTablePtr table, const void *name) +void *virHashSteal(virHashTablePtr table, const char *name) { void *data = virHashLookup(table, name); if (data) { @@ -458,7 +458,7 @@ void *virHashSteal(virHashTablePtr table, const void *name) void * virHashAtomicSteal(virHashAtomicPtr table, - const void *name) + const char *name) { void *data; @@ -500,7 +500,7 @@ virHashSize(const virHashTable *table) * Returns 0 if the removal succeeded and -1 in case of error or not found. */ int -virHashRemoveEntry(virHashTablePtr table, const void *name) +virHashRemoveEntry(virHashTablePtr table, const char *name) { virHashEntryPtr entry; virHashEntryPtr *nextptr; @@ -615,7 +615,7 @@ virHashRemoveSet(virHashTablePtr table, static int _virHashRemoveAllIter(const void *payload G_GNUC_UNUSED, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *data G_GNUC_UNUSED) { return 1; @@ -654,7 +654,7 @@ virHashRemoveAll(virHashTablePtr table) void *virHashSearch(const virHashTable *ctable, virHashSearcher iter, const void *data, - void **name) + char **name) { size_t i; @@ -685,7 +685,7 @@ struct getKeysIter }; static int virHashGetKeysIterator(void *payload, - const void *key, void *data) + const char *key, void *data) { struct getKeysIter *iter = data; @@ -728,7 +728,7 @@ struct virHashEqualData virHashValueComparator compar; }; -static int virHashEqualSearcher(const void *payload, const void *name, +static int virHashEqualSearcher(const void *payload, const char *name, const void *data) { struct virHashEqualData *vhed = (void *)data; diff --git a/src/util/virhash.h b/src/util/virhash.h index a9b022f362..78c7459390 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -40,7 +40,7 @@ typedef void (*virHashDataFree) (void *payload); * * Returns -1 to stop the iteration, e.g. in case of an error */ -typedef int (*virHashIterator) (void *payload, const void *name, void *data); +typedef int (*virHashIterator) (void *payload, const char *name, void *data); /** * virHashSearcher: * @payload: the data in the hash @@ -51,7 +51,7 @@ typedef int (*virHashIterator) (void *payload, const void *name, void *data); * Returns 1 if the hash entry is desired, 0 to move * to next entry */ -typedef int (*virHashSearcher) (const void *payload, const void *name, +typedef int (*virHashSearcher) (const void *payload, const char *name, const void *data); /** @@ -64,7 +64,7 @@ typedef int (*virHashSearcher) (const void *payload, const void *name, * * Returns the hash code */ -typedef uint32_t (*virHashKeyCode)(const void *name, +typedef uint32_t (*virHashKeyCode)(const char *name, uint32_t seed); /** * virHashKeyEqual: @@ -75,7 +75,7 @@ typedef uint32_t (*virHashKeyCode)(const void *name, * * Returns true if the keys are equal, false otherwise */ -typedef bool (*virHashKeyEqual)(const void *namea, const void *nameb); +typedef bool (*virHashKeyEqual)(const char *namea, const char *nameb); /** * virHashKeyCopy: * @name: the hash key @@ -86,7 +86,7 @@ typedef bool (*virHashKeyEqual)(const void *namea, const void *nameb); * Returns a copy of @name which will eventually be passed to the * 'virHashKeyFree' callback at the end of its lifetime. */ -typedef void *(*virHashKeyCopy)(const void *name); +typedef char *(*virHashKeyCopy)(const char *name); /** * virHashKeyPrintHuman: * @name: the hash key @@ -97,7 +97,7 @@ typedef void *(*virHashKeyCopy)(const void *name); * Returns a string representation of the key for use in error messages. Caller * promises to always free the returned string. */ -typedef char *(*virHashKeyPrintHuman) (const void *name); +typedef char *(*virHashKeyPrintHuman) (const char *name); /** * virHashKeyFree: @@ -106,7 +106,7 @@ typedef char *(*virHashKeyPrintHuman) (const void *name); * Free any memory associated with the hash * key @name */ -typedef void (*virHashKeyFree)(void *name); +typedef void (*virHashKeyFree)(char *name); /* * Constructor and destructor. @@ -120,19 +120,19 @@ ssize_t virHashSize(const virHashTable *table); * Add a new entry to the hash table. */ int virHashAddEntry(virHashTablePtr table, - const void *name, void *userdata); + const char *name, void *userdata); int virHashUpdateEntry(virHashTablePtr table, - const void *name, + const char *name, void *userdata); int virHashAtomicUpdate(virHashAtomicPtr table, - const void *name, + const char *name, void *userdata); /* * Remove an entry from the hash table. */ int virHashRemoveEntry(virHashTablePtr table, - const void *name); + const char *name); /* * Remove all entries from the hash table. @@ -142,15 +142,15 @@ ssize_t virHashRemoveAll(virHashTablePtr table); /* * Retrieve the userdata. */ -void *virHashLookup(const virHashTable *table, const void *name); -bool virHashHasEntry(const virHashTable *table, const void *name); +void *virHashLookup(const virHashTable *table, const char *name); +bool virHashHasEntry(const virHashTable *table, const char *name); /* * Retrieve & remove the userdata. */ -void *virHashSteal(virHashTablePtr table, const void *name); +void *virHashSteal(virHashTablePtr table, const char *name); void *virHashAtomicSteal(virHashAtomicPtr table, - const void *name); + const char *name); /* * Get the hash table's key/value pairs and have them optionally sorted. @@ -193,6 +193,6 @@ bool virHashEqual(const virHashTable *table1, int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data); ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void *data); void *virHashSearch(const virHashTable *table, virHashSearcher iter, - const void *data, void **name); + const void *data, char **name); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virHashTable, virHashFree); diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 30d6a19d1d..2731d46dfc 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -683,7 +683,7 @@ struct virLockSpaceRemoveData { static int virLockSpaceRemoveResourcesForOwner(const void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, const void *opaque) { virLockSpaceResourcePtr res = (virLockSpaceResourcePtr)payload; diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index 94e73f3530..2d203e72af 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -52,7 +52,7 @@ static virClassPtr virMacMapClass; static int virMacMapHashFree(void *payload, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { g_strfreev(payload); @@ -197,7 +197,7 @@ virMacMapLoadFile(virMacMapPtr mgr, static int virMACMapHashDumper(void *payload, - const void *name, + const char *name, void *data) { virJSONValuePtr obj = virJSONValueNewObject(); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index da7fd4625a..61935134af 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2732,7 +2732,7 @@ testQemuMonitorCPUInfo(const void *opaque) static int testBlockNodeNameDetectFormat(void *payload, - const void *name, + const char *name, void *opaque) { qemuBlockNodeNameBackingChainDataPtr entry = payload; diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index 839be55665..543a5f7f3f 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -381,7 +381,7 @@ struct _checkOwnerData { static int checkOwner(void *payload, - const void *name, + const char *name, void *opaque) { checkOwnerData *data = opaque; @@ -392,7 +392,7 @@ checkOwner(void *payload, !virStringListHasString(data->paths, name)) { fprintf(stderr, "Path %s wasn't restored back to its original owner\n", - (const char *) name); + name); data->chown_fail = true; } @@ -402,7 +402,7 @@ checkOwner(void *payload, static int printXATTR(void *payload, - const void *name, + const char *name, void *data) { bool *xattr_fail = data; @@ -413,7 +413,7 @@ printXATTR(void *payload, /* Hash table key consists of "$path:$xattr_name", xattr * value is then the value stored in the hash table. */ - printf("key=%s val=%s\n", (const char *) name, (const char *) payload); + printf("key=%s val=%s\n", name, (const char *) payload); return 0; } diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 0ba5b9ed31..ad50aae003 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -46,7 +46,7 @@ testHashInit(void) static int testHashCheckForEachCount(void *payload G_GNUC_UNUSED, - const void *name G_GNUC_UNUSED, + const char *name G_GNUC_UNUSED, void *data G_GNUC_UNUSED) { size_t *count = data; @@ -175,7 +175,7 @@ const int testHashCountRemoveForEachSome = static int testHashRemoveForEachSome(void *payload G_GNUC_UNUSED, - const void *name, + const char *name, void *data) { virHashTablePtr hash = data; @@ -198,7 +198,7 @@ const int testHashCountRemoveForEachAll = 0; static int testHashRemoveForEachAll(void *payload G_GNUC_UNUSED, - const void *name, + const char *name, void *data) { virHashTablePtr hash = data; @@ -266,7 +266,7 @@ testHashSteal(const void *data G_GNUC_UNUSED) static int testHashRemoveSetIter(const void *payload G_GNUC_UNUSED, - const void *name, + const char *name, const void *data) { int *count = (int *) data; @@ -326,7 +326,7 @@ const int testSearchIndex = G_N_ELEMENTS(uuids_subset) / 2; static int testHashSearchIter(const void *payload G_GNUC_UNUSED, - const void *name, + const char *name, const void *data G_GNUC_UNUSED) { return STREQ(uuids_subset[testSearchIndex], name); -- 2.26.2

On Thu, Oct 22, 2020 at 11:35:04AM +0200, Peter Krempa wrote:
All users of virHashTable pass strings as the name/key of the entry. Make this an official requirement by turning the variables to 'const char *'.
For any other case it's better to use glib's GHashTable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/nwfilter_params.c | 2 +- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 6 ++-- src/conf/virdomainobjlist.c | 12 ++++---- src/conf/virinterfaceobj.c | 10 +++--- src/conf/virnetworkobj.c | 16 +++++----- src/conf/virnodedeviceobj.c | 18 +++++------ src/conf/virnwfilterbindingobjlist.c | 4 +-- src/conf/virsecretobj.c | 8 ++--- src/conf/virstorageobj.c | 22 +++++++------- src/hypervisor/virclosecallbacks.c | 2 +- src/locking/lock_daemon.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 ++-- src/nwfilter/nwfilter_gentech_driver.c | 6 ++-- src/qemu/qemu_blockjob.c | 2 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_domain.c | 6 ++-- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_snapshot.c | 4 +-- src/rpc/virnetdaemon.c | 14 ++++----- src/test/test_driver.c | 6 ++-- src/util/virfilecache.c | 2 +- src/util/virhash.c | 42 +++++++++++++------------- src/util/virhash.h | 32 ++++++++++---------- src/util/virlockspace.c | 2 +- src/util/virmacmap.c | 4 +-- tests/qemumonitorjsontest.c | 2 +- tests/qemusecuritymock.c | 8 ++--- tests/virhashtest.c | 10 +++--- 31 files changed, 129 insertions(+), 129 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Since we use virHashTable for string-keyed values only, we can remove all the callbacks which allowed universal keys. Code which wishes to use non-string keys should use glib's GHashTable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhash.c | 68 +++++++--------------------------------------- src/util/virhash.h | 54 ------------------------------------ 2 files changed, 10 insertions(+), 112 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index f386839d6b..7a20b28379 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -56,11 +56,6 @@ struct _virHashTable { size_t size; size_t nbElems; virHashDataFree dataFree; - virHashKeyCode keyCode; - virHashKeyEqual keyEqual; - virHashKeyCopy keyCopy; - virHashKeyPrintHuman keyPrint; - virHashKeyFree keyFree; }; struct _virHashAtomic { @@ -81,40 +76,10 @@ static int virHashAtomicOnceInit(void) VIR_ONCE_GLOBAL_INIT(virHashAtomic); - -static uint32_t virHashStrCode(const char *name, uint32_t seed) -{ - return virHashCodeGen(name, strlen(name), seed); -} - -static bool virHashStrEqual(const char *namea, const char *nameb) -{ - return STREQ(namea, nameb); -} - -static char *virHashStrCopy(const char *name) -{ - return g_strdup(name); -} - - -static char * -virHashStrPrintHuman(const char *name) -{ - return g_strdup(name); -} - - -static void virHashStrFree(char *name) -{ - VIR_FREE(name); -} - - static size_t virHashComputeKey(const virHashTable *table, const char *name) { - uint32_t value = table->keyCode(name, table->seed); + uint32_t value = virHashCodeGen(name, strlen(name), table->seed); return value % table->size; } @@ -138,11 +103,6 @@ virHashNew(virHashDataFree dataFree) table->size = 32; table->nbElems = 0; table->dataFree = dataFree; - table->keyCode = virHashStrCode; - table->keyEqual = virHashStrEqual; - table->keyCopy = virHashStrCopy; - table->keyPrint = virHashStrPrintHuman; - table->keyFree = virHashStrFree; table->table = g_new0(virHashEntryPtr, table->size); @@ -260,8 +220,7 @@ virHashFree(virHashTablePtr table) if (table->dataFree) table->dataFree(iter->payload); - if (table->keyFree) - table->keyFree(iter->name); + g_free(iter->name); VIR_FREE(iter); iter = next; } @@ -287,20 +246,15 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const char *name, /* Check for duplicate entry */ for (entry = table->table[key]; entry; entry = entry->next) { - if (table->keyEqual(entry->name, name)) { + if (STREQ(entry->name, name)) { if (is_update) { if (table->dataFree) table->dataFree(entry->payload); entry->payload = userdata; return 0; } else { - g_autofree char *keystr = NULL; - - if (table->keyPrint) - keystr = table->keyPrint(name); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Duplicate hash table key '%s'"), NULLSTR(keystr)); + _("Duplicate hash table key '%s'"), name); return -1; } } @@ -309,7 +263,7 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const char *name, } entry = g_new0(virHashEntry, 1); - entry->name = table->keyCopy(name); + entry->name = g_strdup(name); entry->payload = userdata; if (last) @@ -388,7 +342,7 @@ virHashGetEntry(const virHashTable *table, key = virHashComputeKey(table, name); for (entry = table->table[key]; entry; entry = entry->next) { - if (table->keyEqual(entry->name, name)) + if (STREQ(entry->name, name)) return entry; } @@ -510,11 +464,10 @@ virHashRemoveEntry(virHashTablePtr table, const char *name) nextptr = table->table + virHashComputeKey(table, name); for (entry = *nextptr; entry; entry = entry->next) { - if (table->keyEqual(entry->name, name)) { + if (STREQ(entry->name, name)) { if (table->dataFree) table->dataFree(entry->payload); - if (table->keyFree) - table->keyFree(entry->name); + g_free(entry->name); *nextptr = entry->next; VIR_FREE(entry); table->nbElems--; @@ -601,8 +554,7 @@ virHashRemoveSet(virHashTablePtr table, count++; if (table->dataFree) table->dataFree(entry->payload); - if (table->keyFree) - table->keyFree(entry->name); + g_free(entry->name); *nextptr = entry->next; VIR_FREE(entry); table->nbElems--; @@ -669,7 +621,7 @@ void *virHashSearch(const virHashTable *ctable, for (entry = table->table[i]; entry; entry = entry->next) { if (iter(entry->payload, entry->name, data)) { if (name) - *name = table->keyCopy(entry->name); + *name = g_strdup(entry->name); return entry->payload; } } diff --git a/src/util/virhash.h b/src/util/virhash.h index 78c7459390..d7cea75c35 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -54,60 +54,6 @@ typedef int (*virHashIterator) (void *payload, const char *name, void *data); typedef int (*virHashSearcher) (const void *payload, const char *name, const void *data); -/** - * virHashKeyCode: - * @name: the hash key - * @seed: random seed - * - * Compute the hash code corresponding to the key @name, using - * @seed to perturb the hashing algorithm - * - * Returns the hash code - */ -typedef uint32_t (*virHashKeyCode)(const char *name, - uint32_t seed); -/** - * 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 char *namea, const char *nameb); -/** - * virHashKeyCopy: - * @name: the hash key - * - * Create a copy of the hash key, duplicating - * memory allocation where applicable - * - * Returns a copy of @name which will eventually be passed to the - * 'virHashKeyFree' callback at the end of its lifetime. - */ -typedef char *(*virHashKeyCopy)(const char *name); -/** - * virHashKeyPrintHuman: - * @name: the hash key - * - * Get a human readable version of the key for error messages. Caller - * will free the returned string. - * - * Returns a string representation of the key for use in error messages. Caller - * promises to always free the returned string. - */ -typedef char *(*virHashKeyPrintHuman) (const char *name); - -/** - * virHashKeyFree: - * @name: the hash key - * - * Free any memory associated with the hash - * key @name - */ -typedef void (*virHashKeyFree)(char *name); - /* * Constructor and destructor. */ -- 2.26.2

On Thu, Oct 22, 2020 at 11:35:05AM +0200, Peter Krempa wrote:
Since we use virHashTable for string-keyed values only, we can remove all the callbacks which allowed universal keys.
Code which wishes to use non-string keys should use glib's GHashTable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhash.c | 68 +++++++--------------------------------------- src/util/virhash.h | 54 ------------------------------------ 2 files changed, 10 insertions(+), 112 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Nobody uses the return value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhash.c | 8 ++------ src/util/virhash.h | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index 7a20b28379..301e485e69 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -579,15 +579,11 @@ _virHashRemoveAllIter(const void *payload G_GNUC_UNUSED, * * Free the hash @table's contents. The userdata is * deallocated with the function provided at creation time. - * - * Returns the number of items removed on success, -1 on failure */ -ssize_t +void virHashRemoveAll(virHashTablePtr table) { - return virHashRemoveSet(table, - _virHashRemoveAllIter, - NULL); + virHashRemoveSet(table, _virHashRemoveAllIter, NULL); } /** diff --git a/src/util/virhash.h b/src/util/virhash.h index d7cea75c35..029e6e83b7 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -83,7 +83,7 @@ int virHashRemoveEntry(virHashTablePtr table, /* * Remove all entries from the hash table. */ -ssize_t virHashRemoveAll(virHashTablePtr table); +void virHashRemoveAll(virHashTablePtr table); /* * Retrieve the userdata. -- 2.26.2

On Thu, Oct 22, 2020 at 11:35:06AM +0200, Peter Krempa wrote:
Nobody uses the return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhash.c | 8 ++------ src/util/virhash.h | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (4)
-
Bjoern Walk
-
Daniel P. Berrangé
-
Pavel Hrdina
-
Peter Krempa