[PATCH 00/23] virHashFree refactoring and removal

Peter Krempa (23): virDomainNetDefParseXML: Automatically free GHashTable virDomainDeviceValidateAliasImpl: Automatically free GHashTable and remove cleanup libxlLoggerNew: Avoid virHashFree by rearranging code virNWFilterDoInstantiate: Automatically free temporary GHashTable virNWFilterBuildAll: Automatically free temporary GHashTable qemuDomainUpdateMemoryDeviceInfo: Automatically free temporary GHashTable qemuDomainBlocksStatsGather: Automatically free GHashTable and refactor cleanup qemuDomainGetDiskErrors: Automatically free GHashTable qemuMigrationSrcFetchMirrorStats: Automatically free GHashTable qemuRefreshVirtioChannelState: Automatically free GHashTable and refactor cleanup qemuRefreshPRManagerState: Automatically free GHashTable and refactor cleanup qemuProcessWaitForMonitor: Automatically free GHashTable qemuProcessRefreshDisks: Automatically free GHashTable and refactor cleanup qemuProcessRefreshLegacyBlockjobs: Automatically free GHashTable and refactor cleanup qemuCheckpointGetXMLDescUpdateSize: Don't combine automatic freeing with manual nwfilterxml2firewalltest: virNWFilterIncludeDefToRuleInst: Automatically free GHashTable nwfilterxml2firewalltest: testCompareXMLToArgvFiles: Automatically free GHashTable qemumonitorjsontest: testBlockNodeNameDetect: Automatically free GHashTable qemumonitorjsontest: mymain: Automatically free GHashTable qemuxml2arvtest: Use 'g_hash_table_unref' for clearing the qapi schema cache Switch away from virHashFree util: virhash: Remove 'virHashFree' util: virhash: Replace 'virHashDataFree' by 'GDestroyNotify' src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 6 +-- src/conf/domain_validate.c | 13 ++--- src/conf/nwfilter_conf.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 3 +- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 4 +- src/conf/virinterfaceobj.c | 2 +- src/conf/virnetworkobj.c | 4 +- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingdef.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 | 9 ++-- src/locking/lock_daemon.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +-- src/nwfilter/nwfilter_gentech_driver.c | 11 ++-- src/nwfilter/nwfilter_learnipaddr.c | 7 +-- src/qemu/qemu_checkpoint.c | 16 +++--- src/qemu/qemu_domain.c | 9 ++-- src/qemu/qemu_driver.c | 26 ++++------ src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 69 ++++++++------------------ src/rpc/virnetdaemon.c | 2 +- src/security/security_selinux.c | 6 +-- src/util/virfilecache.c | 2 +- src/util/virhash.c | 25 ++-------- src/util/virhash.h | 13 +---- src/util/virlockspace.c | 2 +- src/util/virmacmap.c | 2 +- src/util/virobject.c | 4 +- src/util/virsystemd.c | 2 +- tests/nwfilterxml2firewalltest.c | 8 ++- tests/qemumonitorjsontest.c | 32 ++++++------ tests/qemusecuritymock.c | 7 ++- tests/qemuxml2argvtest.c | 2 +- 42 files changed, 123 insertions(+), 207 deletions(-) -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f88405ab02..25e504a99a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10244,7 +10244,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr vlan_node = NULL; xmlNodePtr bandwidth_node = NULL; xmlNodePtr tmpNode; - GHashTable *filterparams = NULL; + g_autoptr(GHashTable) filterparams = NULL; virDomainActualNetDef *actual = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) virDomainChrSourceReconnectDef reconnect = {0}; @@ -10431,7 +10431,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if ((filterref_node = virXPathNode("./filterref", ctxt))) { filter = virXMLPropString(filterref_node, "filter"); - virHashFree(filterparams); filterparams = virNWFilterParseParamAttributes(filterref_node); } @@ -10960,7 +10959,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, cleanup: virDomainActualNetDefFree(actual); - virHashFree(filterparams); return def; error: -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f88405ab02..25e504a99a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10244,7 +10244,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr vlan_node = NULL; xmlNodePtr bandwidth_node = NULL; xmlNodePtr tmpNode; - GHashTable *filterparams = NULL; + g_autoptr(GHashTable) filterparams = NULL; virDomainActualNetDef *actual = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) virDomainChrSourceReconnectDef reconnect = {0}; @@ -10431,7 +10431,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
if ((filterref_node = virXPathNode("./filterref", ctxt))) { filter = virXMLPropString(filterref_node, "filter"); - virHashFree(filterparams);
This virHashFree is called with a NULL parameter since the refactor to xpath in commit fdd06824e3a618ca33752e0439bbd5b2d9da1b0d
filterparams = virNWFilterParseParamAttributes(filterref_node); }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

After the conversion to g_autofree, the cleanup label is no longer needed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_validate.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 80401cf8c7..a4271f1247 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1360,28 +1360,23 @@ static int virDomainDeviceValidateAliasImpl(const virDomainDef *def, virDomainDeviceDef *dev) { - GHashTable *aliases = NULL; + g_autoptr(GHashTable) aliases = NULL; virDomainDeviceInfo *info = virDomainDeviceGetInfo(dev); - int ret = -1; if (!info || !info->alias) return 0; if (virDomainDefValidateAliases(def, &aliases) < 0) - goto cleanup; + return -1; if (virHashLookup(aliases, info->alias)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("non unique alias detected: %s"), info->alias); - goto cleanup; + return -1; } - ret = 0; - cleanup: - - virHashFree(aliases); - return ret; + return 0; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
After the conversion to g_autofree, the cleanup label is no longer needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_validate.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Allocate the hash table only after the log file is opened so that we don't need to deallocate it on failure. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libxl/libxl_logger.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c index f7b5c8ee16..4692578124 100644 --- a/src/libxl/libxl_logger.c +++ b/src/libxl/libxl_logger.c @@ -149,14 +149,13 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel) break; } logger.logDir = logDir; - logger.files = virHashNew(libxlLoggerFileFree); path = g_strdup_printf("%s/libxl-driver.log", logDir); - if ((logger.defaultLogFile = fopen(path, "a")) == NULL) { - virHashFree(logger.files); + if ((logger.defaultLogFile = fopen(path, "a")) == NULL) return NULL; - } + + logger.files = virHashNew(libxlLoggerFileFree); return XTL_NEW_LOGGER(libvirt, logger); } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Allocate the hash table only after the log file is opened so that we don't need to deallocate it on failure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libxl/libxl_logger.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index ecba16d55c..55c7571ea5 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -493,8 +493,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriver *techdriver, virNWFilterVarValue *lv; const char *learning; bool reportIP = false; - - GHashTable *missing_vars = virHashNew(virNWFilterVarValueHashFree); + g_autoptr(GHashTable) missing_vars = virHashNew(virNWFilterVarValueHashFree); memset(&inst, 0, sizeof(inst)); @@ -593,7 +592,6 @@ virNWFilterDoInstantiate(virNWFilterTechDriver *techdriver, error: virNWFilterInstReset(&inst); - virHashFree(missing_vars); return rc; -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 55c7571ea5..ea1f4f4092 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -979,7 +979,8 @@ virNWFilterBuildAll(virNWFilterDriverState *driver, VIR_DEBUG("Build all filters newFilters=%d", newFilters); if (newFilters) { - data.skipInterfaces = virHashNew(NULL); + g_autoptr(GHashTable) skipInterfaces = virHashNew(NULL); + data.skipInterfaces = skipInterfaces; data.step = STEP_APPLY_NEW; if (virNWFilterBindingObjListForEach(driver->bindings, @@ -998,8 +999,6 @@ virNWFilterBuildAll(virNWFilterDriverState *driver, virNWFilterBuildIter, &data); } - - virHashFree(data.skipInterfaces); } else { data.step = STEP_APPLY_CURRENT; if (virNWFilterBindingObjListForEach(driver->bindings, -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1f45b30975..3dfc79f61e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8198,7 +8198,7 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriver *driver, int asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; - GHashTable *meminfo = NULL; + g_autoptr(GHashTable) meminfo = NULL; int rc; size_t i; @@ -8210,10 +8210,8 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriver *driver, rc = qemuMonitorGetMemoryDeviceInfo(priv->mon, &meminfo); - if (qemuDomainObjExitMonitor(driver, vm) < 0) { - virHashFree(meminfo); + if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; - } if (rc < 0) return -1; @@ -8247,7 +8245,6 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriver *driver, } } - virHashFree(meminfo); return 0; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

No need for the cleanup section once we switch to g_autoptr. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4e680bc0a7..5330a1d3b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9975,24 +9975,23 @@ qemuDomainBlocksStatsGather(virQEMUDriver *driver, qemuDomainObjPrivate *priv = vm->privateData; bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); virDomainDiskDef *disk = NULL; - GHashTable *blockstats = NULL; + g_autoptr(GHashTable) blockstats = NULL; qemuBlockStats *stats; size_t i; int nstats; int rc = 0; const char *entryname = NULL; - int ret = -1; if (*path) { if (!(disk = virDomainDiskByName(vm->def, path, false))) { virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); - goto cleanup; + return -1; } if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("block stats are not supported for vhostuser disk")); - goto cleanup; + return -1; } if (blockdev && QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName) { @@ -10001,7 +10000,7 @@ qemuDomainBlocksStatsGather(virQEMUDriver *driver, if (!disk->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing disk device alias name for %s"), disk->dst); - goto cleanup; + return -1; } entryname = disk->info.alias; @@ -10019,7 +10018,7 @@ qemuDomainBlocksStatsGather(virQEMUDriver *driver, } if (qemuDomainObjExitMonitor(driver, vm) < 0 || nstats < 0 || rc < 0) - goto cleanup; + return -1; *retstats = g_new0(qemuBlockStats, 1); @@ -10027,7 +10026,7 @@ qemuDomainBlocksStatsGather(virQEMUDriver *driver, if (!(stats = virHashLookup(blockstats, entryname))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find statistics for device '%s'"), entryname); - goto cleanup; + return -1; } **retstats = *stats; @@ -10063,18 +10062,14 @@ qemuDomainBlocksStatsGather(virQEMUDriver *driver, if (!(stats = virHashLookup(blockstats, entryname))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find statistics for device '%s'"), entryname); - goto cleanup; + return -1; } qemuDomainBlockStatsGatherTotals(stats, *retstats); } } - ret = nstats; - - cleanup: - virHashFree(blockstats); - return ret; + return nstats; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
No need for the cleanup section once we switch to g_autoptr.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5330a1d3b7..4fcd6190d1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16439,7 +16439,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom, virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; qemuDomainObjPrivate *priv; - GHashTable *table = NULL; + g_autoptr(GHashTable) table = NULL; bool blockdev = false; int ret = -1; size_t i; @@ -16501,7 +16501,6 @@ qemuDomainGetDiskErrors(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virHashFree(table); if (ret < 0) { for (i = 0; i < n; i++) VIR_FREE(errors[i].disk); -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8001792f5a..8fb9a3214c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -6237,7 +6237,7 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriver *driver, size_t i; qemuDomainObjPrivate *priv = vm->privateData; bool nbd = false; - GHashTable *blockinfo = NULL; + g_autoptr(GHashTable) blockinfo = NULL; qemuDomainMirrorStats *stats = &jobInfo->mirrorStats; for (i = 0; i < vm->def->ndisks; i++) { @@ -6274,6 +6274,5 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriver *driver, stats->total += data->end; } - virHashFree(blockinfo); return 0; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c355a39e15..8dface7ec0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2245,25 +2245,19 @@ qemuRefreshVirtioChannelState(virQEMUDriver *driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; - GHashTable *info = NULL; - int ret = -1; + g_autoptr(GHashTable) info = NULL; + int rc; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto cleanup; - - ret = qemuMonitorGetChardevInfo(priv->mon, &info); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; + return -1; - if (ret < 0) - goto cleanup; + rc = qemuMonitorGetChardevInfo(priv->mon, &info); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; qemuProcessRefreshChannelVirtioState(driver, vm, info, false); - ret = 0; - cleanup: - virHashFree(info); - return ret; + return 0; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8dface7ec0..2fdd5b95e2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2291,26 +2291,19 @@ qemuRefreshPRManagerState(virQEMUDriver *driver, virDomainObj *vm) { qemuDomainObjPrivate *priv = vm->privateData; - GHashTable *info = NULL; - int ret = -1; + g_autoptr(GHashTable) info = NULL; + int rc; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER) || !qemuDomainDefHasManagedPR(vm)) return 0; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorGetPRManagerInfo(priv->mon, &info); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - - if (ret < 0) - goto cleanup; - - ret = qemuProcessRefreshPRManagerState(vm, info); + rc = qemuMonitorGetPRManagerInfo(priv->mon, &info); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; - cleanup: - virHashFree(info); - return ret; + return qemuProcessRefreshPRManagerState(vm, info); } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2fdd5b95e2..d1b34612ee 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2392,7 +2392,7 @@ qemuProcessWaitForMonitor(virQEMUDriver *driver, qemuDomainLogContext *logCtxt) { int ret = -1; - GHashTable *info = NULL; + g_autoptr(GHashTable) info = NULL; qemuDomainObjPrivate *priv = vm->privateData; bool retry = true; @@ -2425,8 +2425,6 @@ qemuProcessWaitForMonitor(virQEMUDriver *driver, } cleanup: - virHashFree(info); - if (logCtxt && kill(vm->pid, 0) == -1 && errno == ESRCH) { qemuProcessReportLogError(logCtxt, _("process exited while connecting to monitor")); -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d1b34612ee..382d562fb1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8316,18 +8316,17 @@ qemuProcessRefreshDisks(virQEMUDriver *driver, { qemuDomainObjPrivate *priv = vm->privateData; bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); - GHashTable *table = NULL; - int ret = -1; + g_autoptr(GHashTable) table = NULL; size_t i; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { table = qemuMonitorGetBlockInfo(priv->mon); if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto cleanup; + return -1; } if (!table) - goto cleanup; + return -1; for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDef *disk = vm->def->disks[i]; @@ -8358,11 +8357,7 @@ qemuProcessRefreshDisks(virQEMUDriver *driver, diskpriv->tray = info->tray; } - ret = 0; - - cleanup: - virHashFree(table); - return ret; + return 0; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 382d562fb1..5bbd03210b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8540,22 +8540,17 @@ static int qemuProcessRefreshLegacyBlockjobs(virQEMUDriver *driver, virDomainObj *vm) { - GHashTable *blockJobs = NULL; - int ret = -1; + g_autoptr(GHashTable) blockJobs = NULL; qemuDomainObjEnterMonitor(driver, vm); blockJobs = qemuMonitorGetAllBlockJobInfo(qemuDomainGetMonitor(vm), true); if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockJobs) - goto cleanup; + return -1; if (virHashForEach(blockJobs, qemuProcessRefreshLegacyBlockjob, vm) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - virHashFree(blockJobs); - return ret; + return 0; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

'blockNamedNodeData' is declared for automatic freeing but we also free it manually and reuse which is a code pattern we don't normally allow. Rewrite the code to have actually two separate hash tables. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 4902585e5d..72f34ec86d 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -647,7 +647,8 @@ qemuCheckpointGetXMLDescUpdateSize(virDomainObj *vm, { qemuDomainObjPrivate *priv = vm->privateData; virQEMUDriver *driver = priv->driver; - g_autoptr(GHashTable) blockNamedNodeData = NULL; + g_autoptr(GHashTable) nodedataMerge = NULL; + g_autoptr(GHashTable) nodedataStats = NULL; g_autofree struct qemuCheckpointDiskMap *diskmap = NULL; g_autoptr(virJSONValue) recoveractions = NULL; g_autoptr(virJSONValue) mergeactions = virJSONValueNewArray(); @@ -663,7 +664,7 @@ qemuCheckpointGetXMLDescUpdateSize(virDomainObj *vm, if (virDomainObjCheckActive(vm) < 0) goto endjob; - if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) + if (!(nodedataMerge = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) goto endjob; /* enumerate disks relevant for the checkpoint which are also present in the @@ -683,7 +684,7 @@ qemuCheckpointGetXMLDescUpdateSize(virDomainObj *vm, if (!(domdisk = virDomainDiskByTarget(vm->def, chkdisk->name))) continue; - if (!qemuBlockBitmapChainIsValid(domdisk->src, chkdef->parent.name, blockNamedNodeData)) + if (!qemuBlockBitmapChainIsValid(domdisk->src, chkdef->parent.name, nodedataMerge)) continue; diskmap[ndisks].chkdisk = chkdisk; @@ -702,7 +703,7 @@ qemuCheckpointGetXMLDescUpdateSize(virDomainObj *vm, g_autoptr(virJSONValue) actions = NULL; /* possibly delete leftovers from previous cases */ - if (qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, domdisk->src, + if (qemuBlockNamedNodeDataGetBitmapByName(nodedataMerge, domdisk->src, "libvirt-tmp-size-xml")) { if (!recoveractions) recoveractions = virJSONValueNewArray(); @@ -715,7 +716,7 @@ qemuCheckpointGetXMLDescUpdateSize(virDomainObj *vm, if (qemuBlockGetBitmapMergeActions(domdisk->src, NULL, domdisk->src, chkdef->parent.name, "libvirt-tmp-size-xml", - NULL, &actions, blockNamedNodeData) < 0) + NULL, &actions, nodedataMerge) < 0) goto endjob; if (virJSONValueArrayConcat(mergeactions, actions) < 0) @@ -739,8 +740,7 @@ qemuCheckpointGetXMLDescUpdateSize(virDomainObj *vm, goto endjob; /* now do a final refresh */ - virHashFree(blockNamedNodeData); - if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) + if (!(nodedataStats = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) goto endjob; qemuDomainObjEnterMonitor(driver, vm); @@ -756,7 +756,7 @@ qemuCheckpointGetXMLDescUpdateSize(virDomainObj *vm, virDomainDiskDef *domdisk = diskmap[i].domdisk; qemuBlockNamedNodeDataBitmap *bitmap; - if ((bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, domdisk->src, + if ((bitmap = qemuBlockNamedNodeDataGetBitmapByName(nodedataStats, domdisk->src, "libvirt-tmp-size-xml"))) { chkdisk->size = bitmap->dirtybytes; chkdisk->sizeValid = true; -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
'blockNamedNodeData' is declared for automatic freeing but we also free it manually and reuse which is a code pattern we don't normally allow.
Rewrite the code to have actually two separate hash tables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 3e8436a98c..6ab62f60c9 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -227,7 +227,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterIncludeDef *inc, GHashTable *vars, virNWFilterInst *inst) { - GHashTable *tmpvars = NULL; + g_autoptr(GHashTable) tmpvars = NULL; int ret = -1; g_autofree char *xml = NULL; @@ -248,7 +248,6 @@ virNWFilterIncludeDefToRuleInst(virNWFilterIncludeDef *inc, cleanup: if (ret < 0) virNWFilterInstReset(inst); - virHashFree(tmpvars); return ret; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 6ab62f60c9..315afddca0 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -350,7 +350,7 @@ static int testCompareXMLToArgvFiles(const char *xml, { g_autofree char *actualargv = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - GHashTable *vars = virHashNew(virNWFilterVarValueHashFree); + g_autoptr(GHashTable) vars = virHashNew(virNWFilterVarValueHashFree); virNWFilterInst inst; int ret = -1; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); @@ -381,7 +381,6 @@ static int testCompareXMLToArgvFiles(const char *xml, cleanup: virNWFilterInstReset(&inst); - virHashFree(vars); return ret; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Additionally we no longer need the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index bcf5caa9a4..91a9193f2a 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2490,24 +2490,23 @@ testBlockNodeNameDetect(const void *opaque) g_autofree char *actual = NULL; g_autoptr(virJSONValue) namedNodesJson = NULL; g_autoptr(virJSONValue) blockstatsJson = NULL; - GHashTable *nodedata = NULL; + g_autoptr(GHashTable) nodedata = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - int ret = -1; resultFile = g_strdup_printf("%s/%s%s.result", abs_srcdir, pathprefix, testname); if (!(namedNodesJson = virTestLoadFileJSON(pathprefix, testname, "-named-nodes.json", NULL))) - goto cleanup; + return -1; if (!(blockstatsJson = virTestLoadFileJSON(pathprefix, testname, "-blockstats.json", NULL))) - goto cleanup; + return -1; if (!(nodedata = qemuBlockNodeNameGetBackingChain(namedNodesJson, blockstatsJson))) - goto cleanup; + return -1; virHashForEachSorted(nodedata, testBlockNodeNameDetectFormat, &buf); @@ -2516,14 +2515,9 @@ testBlockNodeNameDetect(const void *opaque) actual = virBufferContentAndReset(&buf); if (virTestCompareToFile(actual, resultFile) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virHashFree(nodedata); + return -1; - return ret; + return 0; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Additionally we no longer need the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Use separate automatically cleared variables for the x86_64 and s390 versions of the QAPI schema. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 91a9193f2a..4c882fa5d3 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2890,6 +2890,8 @@ mymain(void) int ret = 0; virQEMUDriver driver; testQemuMonitorJSONSimpleFuncData simpleFunc; + g_autoptr(GHashTable) qapischema_x86_64 = NULL; + g_autoptr(GHashTable) qapischema_s390x = NULL; struct testQAPISchemaData qapiData; g_autoptr(virJSONValue) metaschema = NULL; g_autofree char *metaschemastr = NULL; @@ -2899,12 +2901,14 @@ mymain(void) virEventRegisterDefaultImpl(); - if (!(qapiData.schema = testQEMUSchemaLoadLatest("x86_64"))) { - VIR_TEST_VERBOSE("failed to load qapi schema"); + if (!(qapischema_x86_64 = testQEMUSchemaLoadLatest("x86_64"))) { + VIR_TEST_VERBOSE("failed to load x86_64 qapi schema"); ret = -1; goto cleanup; } + qapiData.schema = qapischema_x86_64; + #define DO_TEST(name) \ do { \ testGenericData data = { driver.xmlopt, qapiData.schema }; \ @@ -3171,18 +3175,18 @@ mymain(void) #undef DO_TEST_QUERY_JOBS - virHashFree(qapiData.schema); - if (!(qapiData.schema = testQEMUSchemaLoadLatest("s390x"))) { + if (!(qapischema_s390x = testQEMUSchemaLoadLatest("s390x"))) { VIR_TEST_VERBOSE("failed to load qapi schema for s390x"); ret = -1; goto cleanup; } + qapiData.schema = qapischema_s390x; + DO_TEST(qemuMonitorJSONGetCPUModelComparison); DO_TEST(qemuMonitorJSONGetCPUModelBaseline); cleanup: - virHashFree(qapiData.schema); qemuTestDriverFree(&driver); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Use separate automatically cleared variables for the x86_64 and s390 versions of the QAPI schema.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b3fa676af7..5e4cd7389c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -823,7 +823,7 @@ mymain(void) int ret = 0; g_autofree char *fakerootdir = NULL; g_autoptr(GHashTable) capslatest = testQemuGetLatestCaps(); - g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) virHashFree); + g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) g_hash_table_unref); g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); struct testQemuConf testConf = { .capslatest = capslatest, .capscache = capscache, -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
s/arv/argv/ in the commit summary
--- tests/qemuxml2argvtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Use 'g_clear_pointer(&ptr, g_hash_table_unref)' instead. In few instances it allows us to also remove explicit clearing of pointers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 3 +-- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 4 ++-- src/conf/virinterfaceobj.c | 2 +- src/conf/virnetworkobj.c | 4 ++-- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingdef.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/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++--- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 7 ++----- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/security/security_selinux.c | 6 +++--- src/util/virfilecache.c | 2 +- src/util/virhash.c | 2 +- src/util/virlockspace.c | 2 +- src/util/virmacmap.c | 2 +- src/util/virsystemd.c | 2 +- tests/nwfilterxml2firewalltest.c | 2 +- tests/qemusecuritymock.c | 7 +++---- 32 files changed, 45 insertions(+), 50 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index a06721c35d..49745ba881 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1377,7 +1377,7 @@ void virDomainCCWAddressSetFree(virDomainCCWAddressSet *addrs) if (!addrs) return; - virHashFree(addrs->defined); + g_clear_pointer(&addrs->defined, g_hash_table_unref); g_free(addrs); } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 25e504a99a..9be5a94680 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2640,7 +2640,7 @@ virDomainNetDefFree(virDomainNetDef *def) virDomainDeviceInfoClear(&def->info); g_free(def->filter); - virHashFree(def->filterparams); + g_clear_pointer(&def->filterparams, g_hash_table_unref); virNetDevBandwidthFree(def->bandwidth); virNetDevVlanClear(&def->vlan); diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index a3109962af..3fedfdde56 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -323,7 +323,7 @@ virNWFilterIncludeDefFree(virNWFilterIncludeDef *inc) { if (!inc) return; - virHashFree(inc->params); + g_clear_pointer(&inc->params, g_hash_table_unref); g_free(inc->filterref); g_free(inc); } diff --git a/src/conf/nwfilter_ipaddrmap.c b/src/conf/nwfilter_ipaddrmap.c index bc21c80536..e2f123b9d9 100644 --- a/src/conf/nwfilter_ipaddrmap.c +++ b/src/conf/nwfilter_ipaddrmap.c @@ -156,6 +156,5 @@ virNWFilterIPAddrMapInit(void) void virNWFilterIPAddrMapShutdown(void) { - virHashFree(ipAddressMap); - ipAddressMap = NULL; + g_clear_pointer(&ipAddressMap, g_hash_table_unref); } diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 5d6de68427..b5477b03d5 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -295,7 +295,7 @@ void virChrdevFree(virChrdevs *devs) virMutexLock(&devs->lock); virHashForEachSafe(devs->hash, virChrdevFreeClearCallbacks, NULL); - virHashFree(devs->hash); + g_clear_pointer(&devs->hash, g_hash_table_unref); virMutexUnlock(&devs->lock); virMutexDestroy(&devs->lock); diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 17b9c16ae7..60f7eec106 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -282,7 +282,7 @@ virDomainMomentObjListFree(virDomainMomentObjList *moments) { if (!moments) return; - virHashFree(moments->objs); + g_clear_pointer(&moments->objs, g_hash_table_unref); g_free(moments); } diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 063c63cd30..9be1e7ad7d 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -85,8 +85,8 @@ static void virDomainObjListDispose(void *obj) { virDomainObjList *doms = obj; - virHashFree(doms->objs); - virHashFree(doms->objsName); + g_clear_pointer(&doms->objs, g_hash_table_unref); + g_clear_pointer(&doms->objsName, g_hash_table_unref); } diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index daac74e88c..9d75be6425 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -345,7 +345,7 @@ virInterfaceObjListDispose(void *obj) { virInterfaceObjList *interfaces = obj; - virHashFree(interfaces->objsName); + g_clear_pointer(&interfaces->objsName, g_hash_table_unref); } diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index f1baffc516..41c7dcba5c 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -464,7 +464,7 @@ virNetworkObjDispose(void *opaque) { virNetworkObj *obj = opaque; - virHashFree(obj->ports); + g_clear_pointer(&obj->ports, g_hash_table_unref); virNetworkDefFree(obj->def); virNetworkDefFree(obj->newDef); virBitmapFree(obj->classIdMap); @@ -477,7 +477,7 @@ virNetworkObjListDispose(void *opaque) { virNetworkObjList *nets = opaque; - virHashFree(nets->objs); + g_clear_pointer(&nets->objs, g_hash_table_unref); } diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 15898a6aa7..2e4ef2df3c 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -453,7 +453,7 @@ virNodeDeviceObjListDispose(void *obj) { virNodeDeviceObjList *devs = obj; - virHashFree(devs->objs); + g_clear_pointer(&devs->objs, g_hash_table_unref); } diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 488fdbceab..c55a5d28dc 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -40,7 +40,7 @@ virNWFilterBindingDefFree(virNWFilterBindingDef *def) g_free(def->portdevname); g_free(def->linkdevname); g_free(def->filter); - virHashFree(def->filterparams); + g_clear_pointer(&def->filterparams, g_hash_table_unref); g_free(def); } diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c index 212cca2265..03441c9c1b 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -77,7 +77,7 @@ virNWFilterBindingObjListDispose(void *obj) { virNWFilterBindingObjList *bindings = obj; - virHashFree(bindings->objs); + g_clear_pointer(&bindings->objs, g_hash_table_unref); } diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 3fbee6f6ec..a88f28fdda 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -149,7 +149,7 @@ virSecretObjListDispose(void *obj) { virSecretObjList *secrets = obj; - virHashFree(secrets->objs); + g_clear_pointer(&secrets->objs, g_hash_table_unref); } diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 2c29339b6a..02903ac487 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -183,9 +183,9 @@ virStorageVolObjListDispose(void *opaque) { virStorageVolObjList *vols = opaque; - virHashFree(vols->objsKey); - virHashFree(vols->objsName); - virHashFree(vols->objsPath); + g_clear_pointer(&vols->objsKey, g_hash_table_unref); + g_clear_pointer(&vols->objsName, g_hash_table_unref); + g_clear_pointer(&vols->objsPath, g_hash_table_unref); } @@ -382,8 +382,8 @@ virStoragePoolObjListDispose(void *opaque) { virStoragePoolObjList *pools = opaque; - virHashFree(pools->objs); - virHashFree(pools->objsName); + g_clear_pointer(&pools->objs, g_hash_table_unref); + g_clear_pointer(&pools->objsName, g_hash_table_unref); } diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index e211fd5d80..6712ef4835 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -357,7 +357,7 @@ hypervAddEmbeddedParam(hypervInvokeParamsList *params, void hypervFreeEmbeddedParam(GHashTable *p) { - virHashFree(p); + g_clear_pointer(&p, g_hash_table_unref); } diff --git a/src/hypervisor/virclosecallbacks.c b/src/hypervisor/virclosecallbacks.c index 2578a71f03..84a61b002a 100644 --- a/src/hypervisor/virclosecallbacks.c +++ b/src/hypervisor/virclosecallbacks.c @@ -78,7 +78,7 @@ virCloseCallbacksDispose(void *obj) { virCloseCallbacks *closeCallbacks = obj; - virHashFree(closeCallbacks->list); + g_clear_pointer(&closeCallbacks->list, g_hash_table_unref); } int diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c index 4692578124..d176c2a7c2 100644 --- a/src/libxl/libxl_logger.c +++ b/src/libxl/libxl_logger.c @@ -166,7 +166,7 @@ libxlLoggerFree(libxlLogger *logger) xentoollog_logger *xtl_logger = (xentoollog_logger*)logger; if (logger->defaultLogFile) VIR_FORCE_FCLOSE(logger->defaultLogFile); - virHashFree(logger->files); + g_clear_pointer(&logger->files, g_hash_table_unref); xtl_logger_destroy(xtl_logger); } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index faca4a2485..cf33d9732b 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -89,7 +89,7 @@ virLockDaemonFree(virLockDaemon *lockd) g_mutex_clear(&lockd->lock); virObjectUnref(lockd->dmn); - virHashFree(lockd->lockspaces); + g_clear_pointer(&lockd->lockspaces, g_hash_table_unref); virLockSpaceFree(lockd->defaultLockspace); g_free(lockd); diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index b0297f0741..386e7640c2 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -2080,13 +2080,13 @@ virNWFilterDHCPSnoopShutdown(void) virNWFilterSnoopLock(); virNWFilterSnoopLeaseFileClose(); - virHashFree(virNWFilterSnoopState.ifnameToKey); - virHashFree(virNWFilterSnoopState.snoopReqs); + g_clear_pointer(&virNWFilterSnoopState.ifnameToKey, g_hash_table_unref); + g_clear_pointer(&virNWFilterSnoopState.snoopReqs, g_hash_table_unref); virNWFilterSnoopUnlock(); virNWFilterSnoopActiveLock(); - virHashFree(virNWFilterSnoopState.active); + g_clear_pointer(&virNWFilterSnoopState.active, g_hash_table_unref); virNWFilterSnoopActiveUnlock(); } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index ea1f4f4092..20ecd299bf 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -121,7 +121,7 @@ virNWFilterRuleInstFree(virNWFilterRuleInst *inst) if (!inst) return; - virHashFree(inst->vars); + g_clear_pointer(&inst->vars, g_hash_table_unref); g_free(inst); } diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 2177c5540b..fb552bd1e6 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -805,9 +805,6 @@ virNWFilterLearnShutdown(void) virNWFilterLearnThreadsTerminate(false); - virHashFree(pendingLearnReq); - pendingLearnReq = NULL; - - virHashFree(ifaceLockMap); - ifaceLockMap = NULL; + g_clear_pointer(&pendingLearnReq, g_hash_table_unref); + g_clear_pointer(&ifaceLockMap, g_hash_table_unref); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3dfc79f61e..c79a7570de 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1701,7 +1701,7 @@ qemuDomainObjPrivateFree(void *data) g_clear_pointer(&priv->migSecinfo, qemuDomainSecretInfoFree); qemuDomainMasterKeyFree(priv); - virHashFree(priv->blockjobs); + g_clear_pointer(&priv->blockjobs, g_hash_table_unref); /* This should never be non-NULL if we get here, but just in case... */ if (priv->eventThread) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4fcd6190d1..00a621d9f8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1084,7 +1084,7 @@ qemuStateCleanup(void) virPortAllocatorRangeFree(qemu_driver->migrationPorts); virPortAllocatorRangeFree(qemu_driver->webSocketPorts); virPortAllocatorRangeFree(qemu_driver->remotePorts); - virHashFree(qemu_driver->sharedDevices); + g_clear_pointer(&qemu_driver->sharedDevices, g_hash_table_unref); virObjectUnref(qemu_driver->hostdevMgr); virObjectUnref(qemu_driver->securityManager); virObjectUnref(qemu_driver->domainEventState); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 444fe3dbe7..c946e4a92a 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -110,7 +110,7 @@ virNetDaemonDispose(void *obj) VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd); g_free(dmn->stateStopThread); - virHashFree(dmn->servers); + g_clear_pointer(&dmn->servers, g_hash_table_unref); virJSONValueFree(dmn->srvObject); } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5682d2bb9d..0952431064 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -692,7 +692,7 @@ virSecuritySELinuxLXCInitialize(virSecurityManager *mgr) VIR_FREE(data->domain_context); VIR_FREE(data->file_context); VIR_FREE(data->content_context); - virHashFree(data->mcs); + g_clear_pointer(&data->mcs, g_hash_table_unref); return -1; } @@ -764,7 +764,7 @@ virSecuritySELinuxQEMUInitialize(virSecurityManager *mgr) VIR_FREE(data->alt_domain_context); VIR_FREE(data->file_context); VIR_FREE(data->content_context); - virHashFree(data->mcs); + g_clear_pointer(&data->mcs, g_hash_table_unref); return -1; } @@ -1030,7 +1030,7 @@ virSecuritySELinuxDriverClose(virSecurityManager *mgr) if (data->label_handle) selabel_close(data->label_handle); - virHashFree(data->mcs); + g_clear_pointer(&data->mcs, g_hash_table_unref); VIR_FREE(data->domain_context); VIR_FREE(data->alt_domain_context); diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c index 64348dc1e6..c140f9d5ea 100644 --- a/src/util/virfilecache.c +++ b/src/util/virfilecache.c @@ -77,7 +77,7 @@ virFileCacheDispose(void *obj) g_free(cache->dir); g_free(cache->suffix); - virHashFree(cache->table); + g_clear_pointer(&cache->table, g_hash_table_unref); virFileCachePrivFree(cache); } diff --git a/src/util/virhash.c b/src/util/virhash.c index a9996c9fc0..cc6053f28d 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -112,7 +112,7 @@ virHashAtomicDispose(void *obj) { virHashAtomic *hash = obj; - virHashFree(hash->hash); + g_clear_pointer(&hash->hash, g_hash_table_unref); } diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 46bd922f35..a7f1c2324f 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -477,7 +477,7 @@ void virLockSpaceFree(virLockSpace *lockspace) if (!lockspace) return; - virHashFree(lockspace->resources); + g_clear_pointer(&lockspace->resources, g_hash_table_unref); g_free(lockspace->dir); virMutexDestroy(&lockspace->lock); g_free(lockspace); diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index fb16062cd1..0b7b758c6d 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -62,7 +62,7 @@ virMacMapDispose(void *obj) while (g_hash_table_iter_next(&htitr, NULL, &value)) g_slist_free_full(value, g_free); - virHashFree(mgr->macs); + g_clear_pointer(&mgr->macs, g_hash_table_unref); } diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 99c92b6f52..a86d4c6bb9 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -1108,7 +1108,7 @@ virSystemdActivationFree(virSystemdActivation *act) if (!act) return; - virHashFree(act->fds); + g_clear_pointer(&act->fds, g_hash_table_unref); g_free(act); } diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 315afddca0..857214dde5 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -166,7 +166,7 @@ virNWFilterRuleInstFree(virNWFilterRuleInst *inst) if (!inst) return; - virHashFree(inst->vars); + g_clear_pointer(&inst->vars, g_hash_table_unref); g_free(inst); } diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index f092383b62..778b0561ac 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -502,10 +502,9 @@ void freePaths(void) virMutexLock(&m); init_hash(); - virHashFree(selinux_paths); - virHashFree(chown_paths); - virHashFree(xattr_paths); - selinux_paths = chown_paths = xattr_paths = NULL; + g_clear_pointer(&selinux_paths, g_hash_table_unref); + g_clear_pointer(&chown_paths, g_hash_table_unref); + g_clear_pointer(&xattr_paths, g_hash_table_unref); virMutexUnlock(&m); } -- 2.31.1

On Tue, Nov 30, 2021 at 03:32:05PM +0100, Peter Krempa wrote:
Use 'g_clear_pointer(&ptr, g_hash_table_unref)' instead.
In few instances it allows us to also remove explicit clearing of pointers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 3 +-- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 4 ++-- src/conf/virinterfaceobj.c | 2 +- src/conf/virnetworkobj.c | 4 ++-- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingdef.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/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++--- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 7 ++----- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/security/security_selinux.c | 6 +++--- src/util/virfilecache.c | 2 +- src/util/virhash.c | 2 +- src/util/virlockspace.c | 2 +- src/util/virmacmap.c | 2 +- src/util/virsystemd.c | 2 +- tests/nwfilterxml2firewalltest.c | 2 +- tests/qemusecuritymock.c | 7 +++---- 32 files changed, 45 insertions(+), 50 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index a06721c35d..49745ba881 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1377,7 +1377,7 @@ void virDomainCCWAddressSetFree(virDomainCCWAddressSet *addrs) if (!addrs) return;
- virHashFree(addrs->defined); + g_clear_pointer(&addrs->defined, g_hash_table_unref);
This is in a Free() method, so we should just be calling g_hash_table_unref directly. Likewise for all the other Free and Dispose methods. There's only a couple of places needing a g_clear_pointer because they're touching global variables. 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 Tue, Nov 30, 2021 at 15:14:14 +0000, Daniel P. Berrangé wrote:
On Tue, Nov 30, 2021 at 03:32:05PM +0100, Peter Krempa wrote:
Use 'g_clear_pointer(&ptr, g_hash_table_unref)' instead.
In few instances it allows us to also remove explicit clearing of pointers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 3 +-- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 4 ++-- src/conf/virinterfaceobj.c | 2 +- src/conf/virnetworkobj.c | 4 ++-- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingdef.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/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++--- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 7 ++----- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/security/security_selinux.c | 6 +++--- src/util/virfilecache.c | 2 +- src/util/virhash.c | 2 +- src/util/virlockspace.c | 2 +- src/util/virmacmap.c | 2 +- src/util/virsystemd.c | 2 +- tests/nwfilterxml2firewalltest.c | 2 +- tests/qemusecuritymock.c | 7 +++---- 32 files changed, 45 insertions(+), 50 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index a06721c35d..49745ba881 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1377,7 +1377,7 @@ void virDomainCCWAddressSetFree(virDomainCCWAddressSet *addrs) if (!addrs) return;
- virHashFree(addrs->defined); + g_clear_pointer(&addrs->defined, g_hash_table_unref);
This is in a Free() method, so we should just be calling g_hash_table_unref directly. Likewise for all the other
g_hash_table_unref requires a valid GHashTable pointer to be passed. g_clear_pointer ensures that we don't call it while the freed table would be NULL. Most Free* methods are also used on cleanup paths when the insides of the struct may still be NULL and IMO it's not worth analyzing the code and neither converting to: if (htptr) g_hash_table_unref(htptr); for the instances where setting the variable to NULL is pointless.

On a Tuesday in 2021, Peter Krempa wrote:
Use 'g_clear_pointer(&ptr, g_hash_table_unref)' instead.
In few instances it allows us to also remove explicit clearing of pointers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 2 +- src/conf/domain_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- src/conf/nwfilter_ipaddrmap.c | 3 +-- src/conf/virchrdev.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 4 ++-- src/conf/virinterfaceobj.c | 2 +- src/conf/virnetworkobj.c | 4 ++-- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingdef.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/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++--- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 7 ++----- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/security/security_selinux.c | 6 +++--- src/util/virfilecache.c | 2 +- src/util/virhash.c | 2 +- src/util/virlockspace.c | 2 +- src/util/virmacmap.c | 2 +- src/util/virsystemd.c | 2 +- tests/nwfilterxml2firewalltest.c | 2 +- tests/qemusecuritymock.c | 7 +++---- 32 files changed, 45 insertions(+), 50 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

The code was converted to stop using this function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virhash.c | 19 ------------------- src/util/virhash.h | 1 - 3 files changed, 21 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b98cb0f66d..dbdc812d0a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2372,7 +2372,6 @@ virHashEqual; virHashForEach; virHashForEachSafe; virHashForEachSorted; -virHashFree; virHashGetItems; virHashHasEntry; virHashLookup; diff --git a/src/util/virhash.c b/src/util/virhash.c index cc6053f28d..ce954e86ec 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -116,25 +116,6 @@ virHashAtomicDispose(void *obj) } -/** - * virHashFree: - * @table: the hash table - * - * Free the hash @table and its contents. The userdata is - * deallocated with function provided at creation time. - * - * Deprecated: consider using g_hash_table_unref instead - */ -void -virHashFree(GHashTable *table) -{ - if (table == NULL) - return; - - g_hash_table_unref(table); -} - - /** * virHashAddEntry: * @table: the hash table diff --git a/src/util/virhash.h b/src/util/virhash.h index 426d835cfc..fcab8454bd 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -52,7 +52,6 @@ typedef int (*virHashSearcher) (const void *payload, const char *name, */ GHashTable *virHashNew(virHashDataFree dataFree) G_GNUC_WARN_UNUSED_RESULT; virHashAtomic *virHashAtomicNew(virHashDataFree dataFree); -void virHashFree(GHashTable *table); ssize_t virHashSize(GHashTable *table); /* -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
The code was converted to stop using this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virhash.c | 19 ------------------- src/util/virhash.h | 1 - 3 files changed, 21 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

We pass through to glib's hash table functions so we can also use glibs function prototype definition. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 2 +- src/util/virhash.c | 4 ++-- src/util/virhash.h | 12 ++---------- src/util/virobject.c | 4 ++-- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 56f0b22b2a..c10ea583fd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2830,7 +2830,7 @@ qemuMonitorJSONBlockGetNamedNodeDataJSON(virJSONValue *nodes) { g_autoptr(GHashTable) ret = NULL; - ret = virHashNew((virHashDataFree) qemuMonitorJSONBlockNamedNodeDataFree); + ret = virHashNew((GDestroyNotify) qemuMonitorJSONBlockNamedNodeDataFree); if (virJSONValueArrayForeachSteal(nodes, qemuMonitorJSONBlockGetNamedNodeDataWorker, diff --git a/src/util/virhash.c b/src/util/virhash.c index ce954e86ec..9fcef621fd 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -83,7 +83,7 @@ virHashTableStringKey(const void *vkey) * Returns the newly created object. */ GHashTable * -virHashNew(virHashDataFree dataFree) +virHashNew(GDestroyNotify dataFree) { ignore_value(virHashTableSeedInitialize()); @@ -92,7 +92,7 @@ virHashNew(virHashDataFree dataFree) virHashAtomic * -virHashAtomicNew(virHashDataFree dataFree) +virHashAtomicNew(GDestroyNotify dataFree) { virHashAtomic *hash; diff --git a/src/util/virhash.h b/src/util/virhash.h index fcab8454bd..8a6b8a7d37 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -15,14 +15,6 @@ typedef struct _virHashAtomic virHashAtomic; * function types: */ -/** - * virHashDataFree: - * @payload: the data in the hash - * - * Callback to free data from a hash. - */ -typedef void (*virHashDataFree) (void *payload); - /** * virHashIterator: * @payload: the data in the hash @@ -50,8 +42,8 @@ typedef int (*virHashSearcher) (const void *payload, const char *name, /* * Constructor and destructor. */ -GHashTable *virHashNew(virHashDataFree dataFree) G_GNUC_WARN_UNUSED_RESULT; -virHashAtomic *virHashAtomicNew(virHashDataFree dataFree); +GHashTable *virHashNew(GDestroyNotify dataFree) G_GNUC_WARN_UNUSED_RESULT; +virHashAtomic *virHashAtomicNew(GDestroyNotify dataFree); ssize_t virHashSize(GHashTable *table); /* diff --git a/src/util/virobject.c b/src/util/virobject.c index 3412985b79..588b41802c 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -604,8 +604,8 @@ void virObjectFreeCallback(void *opaque) * @name: ignored, name of the hash key being deleted * * Provides identical functionality to virObjectUnref, - * but with the signature matching the virHashDataFree - * typedef. + * but with the signature matching the GDestroyNotify + * typedef used with hash tables. */ void virObjectFreeHashData(void *opaque) -- 2.31.1

On a Tuesday in 2021, Peter Krempa wrote:
We pass through to glib's hash table functions so we can also use glibs function prototype definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 2 +- src/util/virhash.c | 4 ++-- src/util/virhash.h | 12 ++---------- src/util/virobject.c | 4 ++-- 4 files changed, 7 insertions(+), 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Tue, Nov 30, 2021 at 03:31:44PM +0100, Peter Krempa wrote:
Peter Krempa (23): virDomainNetDefParseXML: Automatically free GHashTable virDomainDeviceValidateAliasImpl: Automatically free GHashTable and remove cleanup libxlLoggerNew: Avoid virHashFree by rearranging code virNWFilterDoInstantiate: Automatically free temporary GHashTable virNWFilterBuildAll: Automatically free temporary GHashTable qemuDomainUpdateMemoryDeviceInfo: Automatically free temporary GHashTable qemuDomainBlocksStatsGather: Automatically free GHashTable and refactor cleanup qemuDomainGetDiskErrors: Automatically free GHashTable qemuMigrationSrcFetchMirrorStats: Automatically free GHashTable qemuRefreshVirtioChannelState: Automatically free GHashTable and refactor cleanup qemuRefreshPRManagerState: Automatically free GHashTable and refactor cleanup qemuProcessWaitForMonitor: Automatically free GHashTable qemuProcessRefreshDisks: Automatically free GHashTable and refactor cleanup qemuProcessRefreshLegacyBlockjobs: Automatically free GHashTable and refactor cleanup qemuCheckpointGetXMLDescUpdateSize: Don't combine automatic freeing with manual nwfilterxml2firewalltest: virNWFilterIncludeDefToRuleInst: Automatically free GHashTable nwfilterxml2firewalltest: testCompareXMLToArgvFiles: Automatically free GHashTable qemumonitorjsontest: testBlockNodeNameDetect: Automatically free GHashTable qemumonitorjsontest: mymain: Automatically free GHashTable qemuxml2arvtest: Use 'g_hash_table_unref' for clearing the qapi schema cache Switch away from virHashFree util: virhash: Remove 'virHashFree' util: virhash: Replace 'virHashDataFree' by 'GDestroyNotify'
Series Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Thanks for helping to clean up the codebase ;)
participants (4)
-
Daniel P. Berrangé
-
Ján Tomko
-
Martin Kletzander
-
Peter Krempa