util: hash: Use virHashForEachSafe in places which might delete the element

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@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>

diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index 5e5c03d03bf2a2f650dbb9d1edc06379b6ddeca7..f35ae6a93efd411e88e75a0150c3cfdabcc48b47 100644 (file)
--- 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 511bf1d4157c31bcbac32ea532f64b297a804d1f..56658198743bbd97f71e92cfde8c9f5b13e8f81f 100644 (file)
--- 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 9c10090b3217105c6ec3b1c8399b4ee90018aa44..6767fb308e9f9d14194d235a9ee412d6fff61430 100644 (file)
--- 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 8b3eb0f41c42f90f6b771945d4bde9fc02c7cbf2..f5e35dbca4a083617dbcf60ff36859fd9aae3421 100644 (file)
--- 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 194348d0629b93db7ef709c5fce96a5f36a814de..e1b7f0622e341abfaeb6aff740fcaaa2534253ba 100644 (file)
--- 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 4aff62434f9a10116facf7da12b9c2e5c08182f8..d7a90037832ae08c08f208d9ec1548b33bfb826c 100644 (file)
--- 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 e54052985fdc494ed9e94c2eb052f26ed3ff3851..4f8df8fae37d9a331d0d8eaacc00f28b1e3a5e18 100644 (file)
--- 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 93949d8b7a3836b47398c6f6f963ee9a1c814153..4e1d41395f1139096457af0f58cfb66c6e27e554 100644 (file)
--- 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;
     }