[PATCH 08/13] util: hash: Use virHashForEachSafe in places which might delete the element

[Peter Krempa <pkrempa@xxxxxxxxxx>]

Convert all calls to virHashForEach where it's not obvious that the
callback is _not_ deleting the current element from the hash to
virHashForEachSafe which will be deemed safe to do such operation.

Now that no iterator used with virHashForEach deletes current element we
can document that virHashForEach must not touch the hash table in any
way.

Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
 src/conf/virchrdev.c                 | 2 +-
 src/conf/virdomainmomentobjlist.c    | 2 +-
 src/conf/virdomainobjlist.c          | 2 +-
 src/conf/virnetworkobj.c             | 4 ++--
 src/conf/virnwfilterbindingobjlist.c | 2 +-
 src/conf/virstorageobj.c             | 4 ++--
 src/util/virhash.c                   | 4 +++-
 tests/virhashtest.c                  | 2 +-
 8 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index 5e5c03d03b..f35ae6a93e 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -299,7 +299,7 @@ void virChrdevFree(virChrdevsPtr devs)
         return;

     virMutexLock(&devs->lock);
-    virHashForEach(devs->hash, virChrdevFreeClearCallbacks, NULL);
+    virHashForEachSafe(devs->hash, virChrdevFreeClearCallbacks, NULL);
     virHashFree(devs->hash);
     virMutexUnlock(&devs->lock);
     virMutexDestroy(&devs->lock);
diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
index 511bf1d415..5665819874 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -475,7 +475,7 @@ virDomainMomentForEach(virDomainMomentObjListPtr moments,
                        virHashIterator iter,
                        void *data)
 {
-    return virHashForEach(moments->objs, iter, data);
+    return virHashForEachSafe(moments->objs, iter, data);
 }


diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index e9a4b271df..6d8b1eebba 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -838,7 +838,7 @@ virDomainObjListForEach(virDomainObjListPtr doms,
         virObjectRWLockWrite(doms);
     else
         virObjectRWLockRead(doms);
-    virHashForEach(doms->objs, virDomainObjListHelper, &data);
+    virHashForEachSafe(doms->objs, virDomainObjListHelper, &data);
     virObjectRWUnlock(doms);
     return data.ret;
 }
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 46205b163c..6d2f4b6ec9 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1471,7 +1471,7 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
     struct virNetworkObjListForEachHelperData data = {
         .callback = callback, .opaque = opaque, .ret = 0};
     virObjectRWLockRead(nets);
-    virHashForEach(nets->objs, virNetworkObjListForEachHelper, &data);
+    virHashForEachSafe(nets->objs, virNetworkObjListForEachHelper, &data);
     virObjectRWUnlock(nets);
     return data.ret;
 }
@@ -1851,7 +1851,7 @@ virNetworkObjPortForEach(virNetworkObjPtr obj,
                          void *opaque)
 {
     virNetworkObjPortListForEachData data = { iter, opaque, false };
-    virHashForEach(obj->ports, virNetworkObjPortForEachCallback, &data);
+    virHashForEachSafe(obj->ports, virNetworkObjPortForEachCallback, &data);
     if (data.err)
         return -1;
     return 0;
diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c
index 4cbb62abfa..ffcf8faf3a 100644
--- a/src/conf/virnwfilterbindingobjlist.c
+++ b/src/conf/virnwfilterbindingobjlist.c
@@ -365,7 +365,7 @@ virNWFilterBindingObjListForEach(virNWFilterBindingObjListPtr bindings,
         callback, opaque, 0,
     };
     virObjectRWLockRead(bindings);
-    virHashForEach(bindings->objs, virNWFilterBindingObjListHelper, &data);
+    virHashForEachSafe(bindings->objs, virNWFilterBindingObjListHelper, &data);
     virObjectRWUnlock(bindings);
     return data.ret;
 }
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 219594582c..ada53f2e8c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -465,7 +465,7 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools,
     struct _virStoragePoolObjListForEachData data = { .iter = iter,
                                                       .opaque = opaque };

-    virHashForEach(pools->objs, virStoragePoolObjListForEachCb, &data);
+    virHashForEachSafe(pools->objs, virStoragePoolObjListForEachCb, &data);
 }


@@ -753,7 +753,7 @@ virStoragePoolObjForEachVolume(virStoragePoolObjPtr obj,
         .iter = iter, .opaque = opaque };

     virObjectRWLockRead(obj->volumes);
-    virHashForEach(obj->volumes->objsKey, virStoragePoolObjForEachVolumeCb,
+    virHashForEachSafe(obj->volumes->objsKey, virStoragePoolObjForEachVolumeCb,
                    &data);
     virObjectRWUnlock(obj->volumes);
     return 0;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index e54052985f..4f8df8fae3 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -490,7 +490,9 @@ virHashRemoveEntry(virHashTablePtr table, const char *name)
  *
  * The elements are iterated in arbitrary order.
  *
- * virHashForEach, virHashForEachSafe allow the callback to remove the current
+ * virHashForEach prohibits @iter from modifying @table
+ *
+ * virHashForEachSafe allows the callback to remove the current
  * element using virHashRemoveEntry but calling other virHash* functions is
  * prohibited. Note that removing the entry invalidates @key and @payload in
  * the callback.
diff --git a/tests/virhashtest.c b/tests/virhashtest.c
index 93949d8b7a..4e1d41395f 100644
--- a/tests/virhashtest.c
+++ b/tests/virhashtest.c
@@ -218,7 +218,7 @@ testHashRemoveForEach(const void *data)
     if (!(hash = testHashInit()))
         return -1;

-    if (virHashForEach(hash, (virHashIterator) info->data, hash)) {
+    if (virHashForEachSafe(hash, (virHashIterator) info->data, hash)) {
         VIR_TEST_VERBOSE("\nvirHashForEach didn't go through all entries");
         goto cleanup;
     }
-- 
2.26.2