util: hash: Rewrite sorting of elements in virHashGetItems

All but one of the callers either use the list in arbitrary order or
sorted by key. Rewrite the function so that it supports sorting by key
natively and make it return the element count. This in turn allows to
rewrite the only caller to sort by value internally.

This allows to remove multiple sorting functions which were sorting by
key and the function will be also later reused for some hash operations
internally.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>

diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 73160a38a40d957617a521e1f3bac46ced3cbdae..dd2b92c97b5fc9a4310677436372d7713e45f639 100644 (file)
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -755,13 +755,6 @@ virNWFilterParseParamAttributes(xmlNodePtr cur)
 }
 
 
-static int
-virNWFilterFormatParameterNameSorter(const virHashKeyValuePair *a,
-                                     const virHashKeyValuePair *b)
-{
-    return strcmp(a->key, b->key);
-}
-
 int
 virNWFilterFormatParamAttributes(virBufferPtr buf,
                                  virHashTablePtr table,
@@ -779,8 +772,7 @@ virNWFilterFormatParamAttributes(virBufferPtr buf,
         return -1;
     }
 
-    items = virHashGetItems(table,
-                            virNWFilterFormatParameterNameSorter);
+    items = virHashGetItems(table, NULL, true);
     if (!items)
         return -1;
 

diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 1d0d33ec6d7266ac34513d803154b9fa5b80274b..7a66e4810eab5476898b9ed55bd10d1f6b48cfa4 100644 (file)
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -681,7 +681,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri,
 
     /* retrieve parameters out of hash table */
     numKeys = virHashSize(p->embedded.table);
-    items = virHashGetItems(p->embedded.table, NULL);
+    items = virHashGetItems(p->embedded.table, NULL, false);
     if (!items) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Could not read embedded param hash table"));

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 8f16dfd0647998643bc0e347c2a4111522b82221..021792cc69727dba5d1db1786b82665cbe551054 100644 (file)
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -730,7 +730,7 @@ virLockDaemonPreExecRestart(const char *state_file,
     }
 
 
-    tmp = pairs = virHashGetItems(lockDaemon->lockspaces, NULL);
+    tmp = pairs = virHashGetItems(lockDaemon->lockspaces, NULL, false);
     while (tmp && tmp->key) {
         virLockSpacePtr lockspace = (virLockSpacePtr)tmp->value;
 

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index da6f88f13596da6d1eac53e43ae54f78fdcbe37d..8dfc870ab70104ec479aa6270257a386ccb75784 100644 (file)
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3126,9 +3126,12 @@ virNWFilterRuleInstSortPtr(const void *a, const void *b)
 
 
 static int
-ebiptablesFilterOrderSort(const virHashKeyValuePair *a,
-                          const virHashKeyValuePair *b)
+ebiptablesFilterOrderSort(const void *va,
+                          const void *vb)
 {
+    const virHashKeyValuePair *a = va;
+    const virHashKeyValuePair *b = vb;
+
     /* elements' values has been limited to range [-1000, 1000] */
     return *(virNWFilterChainPriority *)a->value -
            *(virNWFilterChainPriority *)b->value;
@@ -3288,13 +3291,15 @@ ebtablesGetSubChainInsts(virHashTablePtr chains,
                          size_t *ninsts)
 {
     g_autofree virHashKeyValuePairPtr filter_names = NULL;
+    size_t nfilter_names;
     size_t i;
 
-    filter_names = virHashGetItems(chains,
-                                   ebiptablesFilterOrderSort);
+    filter_names = virHashGetItems(chains, &nfilter_names, false);
     if (filter_names == NULL)
         return -1;
 
+    qsort(filter_names, nfilter_names, sizeof(*filter_names), ebiptablesFilterOrderSort);
+
     for (i = 0; filter_names[i].key; i++) {
         g_autofree ebtablesSubChainInstPtr inst = NULL;
         enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername(

diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c
index e1c20aff1180dbb55543471a610fd6d134547fc8..c8b81c59a8a3e3c1a079f8250713547d0c9767f5 100644 (file)
--- a/src/qemu/qemu_interop_config.c
+++ b/src/qemu/qemu_interop_config.c
@@ -79,13 +79,6 @@ qemuBuildFileList(virHashTablePtr files, const char *dir)
     return 0;
 }
 
-static int
-qemuConfigFilesSorter(const virHashKeyValuePair *a,
-                      const virHashKeyValuePair *b)
-{
-    return strcmp(a->key, b->key);
-}
-
 #define QEMU_SYSTEM_LOCATION PREFIX "/share/qemu"
 #define QEMU_ETC_LOCATION SYSCONFDIR "/qemu"
 
@@ -141,7 +134,7 @@ qemuInteropFetchConfigs(const char *name,
     if (virHashSize(files) == 0)
         return 0;
 
-    if (!(pairs = virHashGetItems(files, qemuConfigFilesSorter)))
+    if (!(pairs = virHashGetItems(files, NULL, true)))
         return -1;
 
     for (tmp = pairs; tmp->key; tmp++) {

diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index ce13f0d92764700193b516867074a17c0d116854..525cbb3ce672e4012443726ae4f73f3127b3fecb 100644 (file)
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -378,15 +378,6 @@ virNetDaemonNewPostExecRestart(virJSONValuePtr object,
 }
 
 
-static int
-daemonServerCompare(const virHashKeyValuePair *a, const virHashKeyValuePair *b)
-{
-    const char *as = a->key;
-    const char *bs = b->key;
-
-    return strcmp(as, bs);
-}
-
 virJSONValuePtr
 virNetDaemonPreExecRestart(virNetDaemonPtr dmn)
 {
@@ -402,7 +393,7 @@ virNetDaemonPreExecRestart(virNetDaemonPtr dmn)
         goto error;
     }
 
-    if (!(srvArray = virHashGetItems(dmn->servers, daemonServerCompare)))
+    if (!(srvArray = virHashGetItems(dmn->servers, NULL, true)))
         goto error;
 
     for (i = 0; srvArray[i].key; i++) {

diff --git a/src/util/virhash.c b/src/util/virhash.c
index b30db8a87a744fba792c914666e31ac6c1326d53..f64499071236d28394da973c9411c1018b2fcb85 100644 (file)
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -626,49 +626,63 @@ void *virHashSearch(const virHashTable *ctable,
     return NULL;
 }
 
-struct getKeysIter
-{
-    virHashKeyValuePair *sortArray;
-    size_t arrayIdx;
+
+struct virHashGetItemsIteratorData {
+    virHashKeyValuePair *items;
+    size_t i;
 };
 
-static int virHashGetKeysIterator(void *payload,
-                                  const char *key, void *data)
+
+static int
+virHashGetItemsIterator(void *payload,
+                        const char *key,
+                        void *opaque)
 {
-    struct getKeysIter *iter = data;
+    struct virHashGetItemsIteratorData *data = opaque;
 
-    iter->sortArray[iter->arrayIdx].key = key;
-    iter->sortArray[iter->arrayIdx].value = payload;
+    data->items[data->i].key = key;
+    data->items[data->i].value = payload;
 
-    iter->arrayIdx++;
+    data->i++;
     return 0;
 }
 
-typedef int (*qsort_comp)(const void *, const void *);
 
-virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table,
-                                       virHashKeyComparator compar)
+static int
+virHashGetItemsKeySorter(const void *va,
+                         const void *vb)
 {
-    ssize_t numElems = virHashSize(table);
-    struct getKeysIter iter = {
-        .arrayIdx = 0,
-        .sortArray = NULL,
-    };
+    const virHashKeyValuePair *a = va;
+    const virHashKeyValuePair *b = vb;
 
-    if (numElems < 0)
-        return NULL;
+    return strcmp(a->key, b->key);
+}
+
+
+virHashKeyValuePairPtr
+virHashGetItems(virHashTablePtr table,
+                size_t *nitems,
+                bool sortKeys)
+{
+    size_t dummy;
+    struct virHashGetItemsIteratorData data = { .items = NULL, .i = 0 };
 
-    iter.sortArray = g_new0(virHashKeyValuePair, numElems + 1);
+    if (!nitems)
+        nitems = &dummy;
 
-    virHashForEach(table, virHashGetKeysIterator, &iter);
+    *nitems = virHashSize(table);
 
-    if (compar)
-        qsort(&iter.sortArray[0], numElems, sizeof(iter.sortArray[0]),
-              (qsort_comp)compar);
+    data.items = g_new0(virHashKeyValuePair, *nitems + 1);
 
-    return iter.sortArray;
+    virHashForEach(table, virHashGetItemsIterator, &data);
+
+    if (sortKeys)
+        qsort(data.items, *nitems, sizeof(* data.items), virHashGetItemsKeySorter);
+
+    return data.items;
 }
 
+
 struct virHashEqualData
 {
     bool equal;

diff --git a/src/util/virhash.h b/src/util/virhash.h
index b2ea5c9a9627a948b1d4d3a20e7e120bec816579..688e1adf4daf21b7b6f3569ce854f334ffd22407 100644 (file)
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -116,10 +116,9 @@ struct _virHashKeyValuePair {
     const void *key;
     const void *value;
 };
-typedef int (*virHashKeyComparator)(const virHashKeyValuePair *,
-                                    const virHashKeyValuePair *);
 virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table,
-                                       virHashKeyComparator compar);
+                                       size_t *nitems,
+                                       bool sortedKeys);
 
 /*
  * Compare two tables for equality: the lookup of a key's value in

diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 2731d46dfc8d8a1ecf8bd6f23570acde477d9e0c..d39f2c026fcad5db42a93d928c8a7dad5dd4972b 100644 (file)
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -435,7 +435,7 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace)
         goto error;
     }
 
-    tmp = pairs = virHashGetItems(lockspace->resources, NULL);
+    tmp = pairs = virHashGetItems(lockspace->resources, NULL, false);
     while (tmp && tmp->value) {
         virLockSpaceResourcePtr res = (virLockSpaceResourcePtr)tmp->value;
         virJSONValuePtr child = virJSONValueNewObject();

diff --git a/tests/virhashtest.c b/tests/virhashtest.c
index 8cc3109929bb6a11ef7c58c1da75484c365149c7..93949d8b7a3836b47398c6f6f963ee9a1c814153 100644 (file)
--- a/tests/virhashtest.c
+++ b/tests/virhashtest.c
@@ -361,13 +361,6 @@ testHashSearch(const void *data G_GNUC_UNUSED)
 }
 
 
-static int
-testHashGetItemsCompKey(const virHashKeyValuePair *a,
-                        const virHashKeyValuePair *b)
-{
-    return strcmp(a->key, b->key);
-}
-
 static int
 testHashGetItems(const void *data G_GNUC_UNUSED)
 {
@@ -389,14 +382,14 @@ testHashGetItems(const void *data G_GNUC_UNUSED)
         goto cleanup;
     }
 
-    if (!(array = virHashGetItems(hash, NULL)) ||
+    if (!(array = virHashGetItems(hash, NULL, false)) ||
         array[3].key || array[3].value) {
         VIR_TEST_VERBOSE("\nfailed to get items with NULL sort");
         goto cleanup;
     }
     VIR_FREE(array);
 
-    if (!(array = virHashGetItems(hash, testHashGetItemsCompKey)) ||
+    if (!(array = virHashGetItems(hash, NULL, true)) ||
         STRNEQ(array[0].key, "a") ||
         STRNEQ(array[0].value, "3") ||
         STRNEQ(array[1].key, "b") ||