I almost wrote a hash value free function that just called VIR_FREE, then realized I couldn't be the first person to do that. Sure enough, it was worth factoring into a common helper routine. Furthermore, in a few places we were passing raw free() as the cleanup function; whereas going through VIR_FREE() ensures that any (temporary) tracing or other memory stress testing that we add to viralloc.c will not be bypassed. * src/util/virhash.h (virHashValueFree): New function. * src/util/virhash.c (virHashValueFree): Implement it. * src/util/virobject.h (virObjectFreeHashData): New function. * src/libvirt_private.syms (virhash.h, virobject.h): Export them. * src/nwfilter/nwfilter_learnipaddr.c (virNWFilterLearnInit): Use common function. * src/qemu/qemu_capabilities.c (virQEMUCapsCacheNew): Likewise. * src/qemu/qemu_command.c (qemuDomainCCWAddressSetCreate): Likewise. * src/qemu/qemu_monitor.c (qemuMonitorGetBlockInfo): Likewise. * src/qemu/qemu_process.c (qemuProcessWaitForMonitor): Likewise. * src/util/virclosecallbacks.c (virCloseCallbacksNew): Likewise. * src/util/virkeyfile.c (virKeyFileParseGroup): Likewise. * tests/qemumonitorjsontest.c (testQemuMonitorJSONqemuMonitorJSONGetBlockInfo): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- It turns out I may end up not using the common helper in my code after all, but the cleanup is still worth posting. src/libvirt_private.syms | 2 ++ src/nwfilter/nwfilter_learnipaddr.c | 9 +-------- src/qemu/qemu_capabilities.c | 9 +-------- src/qemu/qemu_command.c | 8 +------- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_process.c | 6 +----- src/util/virclosecallbacks.c | 11 ++--------- src/util/virhash.c | 9 ++++++++- src/util/virhash.h | 5 ++++- src/util/virkeyfile.c | 9 ++------- src/util/virobject.c | 17 ++++++++++++++++- src/util/virobject.h | 3 ++- tests/qemumonitorjsontest.c | 6 +++--- 13 files changed, 44 insertions(+), 52 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2d12105..6b68a43 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1290,6 +1290,7 @@ virHashSize; virHashSteal; virHashTableSize; virHashUpdateEntry; +virHashValueFree; # util/virhook.h @@ -1628,6 +1629,7 @@ virClassIsDerivedFrom; virClassName; virClassNew; virObjectFreeCallback; +virObjectFreeHashData; virObjectIsClass; virObjectLock; virObjectLockableNew; diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 0eb5e7e..1ffed78 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -188,13 +188,6 @@ virNWFilterLockIface(const char *ifname) } -static void -freeIfaceLock(void *payload, const void *name ATTRIBUTE_UNUSED) -{ - VIR_FREE(payload); -} - - void virNWFilterUnlockIface(const char *ifname) { @@ -818,7 +811,7 @@ virNWFilterLearnInit(void) return -1; } - ifaceLockMap = virHashCreate(0, freeIfaceLock); + ifaceLockMap = virHashCreate(0, virHashValueFree); if (!ifaceLockMap) { virNWFilterLearnShutdown(); return -1; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e1d04ff..2c8ec10 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3288,13 +3288,6 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps) } -static void -virQEMUCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) -{ - virObjectUnref(payload); -} - - virQEMUCapsCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, @@ -3313,7 +3306,7 @@ virQEMUCapsCacheNew(const char *libDir, return NULL; } - if (!(cache->binaries = virHashCreate(10, virQEMUCapsHashDataFree))) + if (!(cache->binaries = virHashCreate(10, virObjectFreeHashData))) goto error; if (VIR_STRDUP(cache->libDir, libDir) < 0) goto error; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 37841d1..cc45828 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1102,12 +1102,6 @@ qemuCCWAdressIncrement(virDomainDeviceCCWAddressPtr addr) return 0; } -static void -qemuDomainCCWAddressSetFreeEntry(void *payload, - const void *name ATTRIBUTE_UNUSED) -{ - VIR_FREE(payload); -} int qemuDomainCCWAddressAssign(virDomainDeviceInfoPtr dev, qemuDomainCCWAddressSetPtr addrs, @@ -1264,7 +1258,7 @@ qemuDomainCCWAddressSetCreate(void) if (VIR_ALLOC(addrs) < 0) goto error; - if (!(addrs->defined = virHashCreate(10, qemuDomainCCWAddressSetFreeEntry))) + if (!(addrs->defined = virHashCreate(10, virHashValueFree))) goto error; /* must use cssid = 0xfe (254) for virtio-ccw devices */ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 205002b..5a5a59b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1678,7 +1678,7 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon) return NULL; } - if (!(table = virHashCreate(32, (virHashDataFree) free))) + if (!(table = virHashCreate(32, virHashValueFree))) return NULL; if (mon->json) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ca9e15c..0afad9d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1871,10 +1871,6 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm, return 0; } -static void qemuProcessFreePtyPath(void *payload, const void *name ATTRIBUTE_UNUSED) -{ - VIR_FREE(payload); -} static int qemuProcessWaitForMonitor(virQEMUDriverPtr driver, @@ -1911,7 +1907,7 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver, * reliable if it's available. * Note that the monitor itself can be on a pty, so we still need to try the * log output method. */ - paths = virHashCreate(0, qemuProcessFreePtyPath); + paths = virHashCreate(0, virHashValueFree); if (paths == NULL) goto cleanup; diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c index d62fd89..4f26172 100644 --- a/src/util/virclosecallbacks.c +++ b/src/util/virclosecallbacks.c @@ -1,7 +1,7 @@ /* * virclosecallbacks.c: Connection close callbacks routines * - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -67,13 +67,6 @@ static int virCloseCallbacksOnceInit(void) VIR_ONCE_GLOBAL_INIT(virCloseCallbacks) -static void -virCloseCallbacksFreeData(void *payload, - const void *name ATTRIBUTE_UNUSED) -{ - VIR_FREE(payload); -} - virCloseCallbacksPtr virCloseCallbacksNew(void) { @@ -85,7 +78,7 @@ virCloseCallbacksNew(void) if (!(closeCallbacks = virObjectLockableNew(virCloseCallbacksClass))) return NULL; - closeCallbacks->list = virHashCreate(5, virCloseCallbacksFreeData); + closeCallbacks->list = virHashCreate(5, virHashValueFree); if (!closeCallbacks->list) { virObjectUnref(closeCallbacks); return NULL; diff --git a/src/util/virhash.c b/src/util/virhash.c index 9ef3554..e3c1880 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -3,7 +3,7 @@ * * Reference: Your favorite introductory book on algorithms * - * Copyright (C) 2005-2013 Red Hat, Inc. + * Copyright (C) 2005-2014 Red Hat, Inc. * Copyright (C) 2000 Bjorn Reese and Daniel Veillard. * * Permission to use, copy, modify, and distribute this software for any @@ -99,6 +99,13 @@ static void virHashStrFree(void *name) } +void +virHashValueFree(void *value, const void *name ATTRIBUTE_UNUSED) +{ + 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 4de9a14..a137137 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -3,7 +3,7 @@ * Description: This module implements the hash table and allocation and * deallocation of domains and connections * - * Copyright (C) 2005-2013 Red Hat, Inc. + * Copyright (C) 2005-2014 Red Hat, Inc. * Copyright (C) 2000 Bjorn Reese and Daniel Veillard. * * Author: Bjorn Reese <bjorn.reese(a)systematic.dk&gt; @@ -184,4 +184,7 @@ ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void void *virHashSearch(const virHashTable *table, virHashSearcher iter, const void *data); +/* Convenience for when VIR_FREE(value) is sufficient as a data freer. */ +void virHashValueFree(void *value, const void *name); + #endif /* ! __VIR_HASH_H__ */ diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 742fb37..da06425 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -1,7 +1,7 @@ /* * virkeyfile.c: "ini"-style configuration file handling * - * Copyright (C) 2012-2013 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -103,11 +103,6 @@ virKeyFileErrorHelper(const char *file, const char *func, size_t line, } -static void virKeyFileValueFree(void *value, const void *name ATTRIBUTE_UNUSED) -{ - VIR_FREE(value); -} - static int virKeyFileParseGroup(virKeyFileParserCtxtPtr ctxt) { int ret = -1; @@ -130,7 +125,7 @@ static int virKeyFileParseGroup(virKeyFileParserCtxtPtr ctxt) NEXT; - if (!(ctxt->group = virHashCreate(10, virKeyFileValueFree))) + if (!(ctxt->group = virHashCreate(10, virHashValueFree))) goto cleanup; if (virHashAddEntry(ctxt->conf->groups, ctxt->groupname, ctxt->group) < 0) diff --git a/src/util/virobject.c b/src/util/virobject.c index c8bc193..6cb84b4 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -1,7 +1,7 @@ /* * virobject.c: libvirt reference counted object * - * Copyright (C) 2012-2013 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -390,3 +390,18 @@ void virObjectFreeCallback(void *opaque) { virObjectUnref(opaque); } + + +/** + * virObjectFreeHashData: + * @opaque: a pointer to a virObject instance + * @name: ignored, name of the hash key being deleted + * + * Provides identical functionality to virObjectUnref, + * but with the signature matching the virHashDataFree + * typedef. + */ +void virObjectFreeHashData(void *opaque, const void *name ATTRIBUTE_UNUSED) +{ + virObjectUnref(opaque); +} diff --git a/src/util/virobject.h b/src/util/virobject.h index d571b5c..ad1f0c1 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -1,7 +1,7 @@ /* * virobject.h: libvirt reference counted object * - * Copyright (C) 2012-2013 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -89,6 +89,7 @@ bool virObjectIsClass(void *obj, ATTRIBUTE_NONNULL(2); void virObjectFreeCallback(void *opaque); +void virObjectFreeHashData(void *opaque, const void *name); void *virObjectLockableNew(virClassPtr klass) ATTRIBUTE_NONNULL(1); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index f80d03e..47d7481 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2013 Red Hat, Inc. + * Copyright (C) 2011-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -1356,8 +1356,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data) if (!test) return -1; - if (!(blockDevices = virHashCreate(32, (virHashDataFree) free)) || - !(expectedBlockDevices = virHashCreate(32, (virHashDataFree) (free)))) + if (!(blockDevices = virHashCreate(32, virHashValueFree)) || + !(expectedBlockDevices = virHashCreate(32, virHashValueFree))) goto cleanup; if (VIR_ALLOC(info) < 0) -- 1.9.0