[PATCH v2 07/12] util: hash: Use virHashForEachSafe in places which might delete the element

[Peter Krempa]

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 <pkre...@redhat.com>
---
 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 9c10090b32..6767fb308e 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -837,7 +837,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 8b3eb0f41c..f5e35dbca4 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1468,7 +1468,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;
 }
@@ -1841,7 +1841,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 194348d062..e1b7f0622e 100644
--- a/src/conf/virnwfilterbindingobjlist.c
+++ b/src/conf/virnwfilterbindingobjlist.c
@@ -364,7 +364,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 4aff62434f..d7a9003783 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