[libvirt] [PATCH] hash: add common utility functions

Eric Blake eblake at redhat.com
Fri Apr 4 23:49:33 UTC 2014


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 at redhat.com>
---

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 at systematic.dk>
@@ -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




More information about the libvir-list mailing list