Enforce rules about modifying hash tables in callbacks

We have the infrastructure to do this, so lets do it.
Also add tests for find and foreach to the testsuite.
Bug 650688

diff --git a/glib/ghash.c b/glib/ghash.c
index 39b5679..e9eace5 100644 (file)
--- a/glib/ghash.c
+++ b/glib/ghash.c
@@ -1228,6 +1228,9 @@ g_hash_table_foreach_remove_or_steal (GHashTable *hash_table,
 {
   guint deleted = 0;
   gint i;
+#ifndef G_DISABLE_ASSERT
+  gint version = hash_table->version;
+#endif
 
   for (i = 0; i < hash_table->size; i++)
     {
@@ -1241,6 +1244,10 @@ g_hash_table_foreach_remove_or_steal (GHashTable *hash_table,
           g_hash_table_remove_node (hash_table, i, notify);
           deleted++;
         }
+
+#ifndef G_DISABLE_ASSERT
+      g_return_val_if_fail (version == hash_table->version, 0);
+#endif
     }
 
   g_hash_table_maybe_resize (hash_table);
@@ -1291,7 +1298,7 @@ g_hash_table_foreach_remove (GHashTable *hash_table,
  * If the function returns %TRUE, then the key/value pair is removed from the
  * #GHashTable, but no key or value destroy functions are called.
  *
- * See #GHashTableIter for an alternative way to loop over the 
+ * See #GHashTableIter for an alternative way to loop over the
  * key/value pairs in the hash table.
  *
  * Return value: the number of key/value pairs removed.
@@ -1329,6 +1336,9 @@ g_hash_table_foreach (GHashTable *hash_table,
                       gpointer    user_data)
 {
   gint i;
+#ifndef G_DISABLE_ASSERT
+  gint version = hash_table->version;
+#endif
 
   g_return_if_fail (hash_table != NULL);
   g_return_if_fail (func != NULL);
@@ -1341,6 +1351,10 @@ g_hash_table_foreach (GHashTable *hash_table,
 
       if (HASH_IS_REAL (node_hash))
         (* func) (node_key, node_value, user_data);
+
+#ifndef G_DISABLE_ASSERT
+      g_return_if_fail (version == hash_table->version);
+#endif
     }
 }
 
@@ -1364,30 +1378,42 @@ g_hash_table_foreach (GHashTable *hash_table,
  * operation issued for all n values in a hash table ends up needing O(n*n)
  * operations).
  *
- * Return value: The value of the first key/value pair is returned, for which
- * func evaluates to %TRUE. If no pair with the requested property is found,
- * %NULL is returned.
+ * Return value: The value of the first key/value pair is returned,
+ *     for which @predicate evaluates to %TRUE. If no pair with the
+ *     requested property is found, %NULL is returned.
  *
  * Since: 2.4
  **/
 gpointer
-g_hash_table_find (GHashTable      *hash_table,
-                   GHRFunc          predicate,
-                   gpointer         user_data)
+g_hash_table_find (GHashTable *hash_table,
+                   GHRFunc     predicate,
+                   gpointer    user_data)
 {
   gint i;
+#ifndef G_DISABLE_ASSERT
+  gint version = hash_table->version;
+#endif
+  gboolean match;
 
   g_return_val_if_fail (hash_table != NULL, NULL);
   g_return_val_if_fail (predicate != NULL, NULL);
 
+  match = FALSE;
+
   for (i = 0; i < hash_table->size; i++)
     {
       guint node_hash = hash_table->hashes[i];
       gpointer node_key = hash_table->keys[i];
       gpointer node_value = hash_table->values[i];
 
-      if (HASH_IS_REAL (node_hash) &&
-          predicate (node_key, node_value, user_data))
+      if (HASH_IS_REAL (node_hash))
+        match = predicate (node_key, node_value, user_data);
+
+#ifndef G_DISABLE_ASSERT
+      g_return_val_if_fail (version == hash_table->version, NULL);
+#endif
+
+      if (match)
         return node_value;
     }
 

diff --git a/glib/tests/hash.c b/glib/tests/hash.c
index 8f3d172..b82120e 100644 (file)
--- a/glib/tests/hash.c
+++ b/glib/tests/hash.c
@@ -853,6 +853,85 @@ test_destroy_modify (void)
   g_hash_table_unref (h);
 }
 
+static gboolean
+find_str (gpointer key, gpointer value, gpointer data)
+{
+  return g_str_equal (key, data);
+}
+
+static void
+test_find (void)
+{
+  GHashTable *hash;
+  const gchar *value;
+
+  hash = g_hash_table_new (g_str_hash, g_str_equal);
+
+  g_hash_table_insert (hash, "a", "A");
+  g_hash_table_insert (hash, "b", "B");
+  g_hash_table_insert (hash, "c", "C");
+  g_hash_table_insert (hash, "d", "D");
+  g_hash_table_insert (hash, "e", "E");
+  g_hash_table_insert (hash, "f", "F");
+
+  value = g_hash_table_find (hash, find_str, "a");
+  g_assert_cmpstr (value, ==, "A");
+
+  value = g_hash_table_find (hash, find_str, "b");
+  g_assert_cmpstr (value, ==, "B");
+
+  value = g_hash_table_find (hash, find_str, "c");
+  g_assert_cmpstr (value, ==, "C");
+
+  value = g_hash_table_find (hash, find_str, "d");
+  g_assert_cmpstr (value, ==, "D");
+
+  value = g_hash_table_find (hash, find_str, "e");
+  g_assert_cmpstr (value, ==, "E");
+
+  value = g_hash_table_find (hash, find_str, "f");
+  g_assert_cmpstr (value, ==, "F");
+
+  value = g_hash_table_find (hash, find_str, "0");
+  g_assert (value == NULL);
+
+  g_hash_table_unref (hash);
+}
+
+gboolean seen_key[6];
+
+static void
+foreach_func (gpointer key, gpointer value, gpointer data)
+{
+  seen_key[((char*)key)[0] - 'a'] = TRUE;
+}
+
+static void
+test_foreach (void)
+{
+  GHashTable *hash;
+  gint i;
+
+  hash = g_hash_table_new (g_str_hash, g_str_equal);
+
+  g_hash_table_insert (hash, "a", "A");
+  g_hash_table_insert (hash, "b", "B");
+  g_hash_table_insert (hash, "c", "C");
+  g_hash_table_insert (hash, "d", "D");
+  g_hash_table_insert (hash, "e", "E");
+  g_hash_table_insert (hash, "f", "F");
+
+  for (i = 0; i < 6; i++)
+    seen_key[i] = FALSE;
+
+  g_hash_table_foreach (hash, foreach_func, NULL);
+
+  for (i = 0; i < 6; i++)
+    g_assert (seen_key[i]);
+
+  g_hash_table_unref (hash);
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -871,6 +950,8 @@ main (int argc, char *argv[])
   g_test_add_func ("/hash/set-ref", set_ref_hash_test);
   g_test_add_func ("/hash/ref", test_hash_ref);
   g_test_add_func ("/hash/remove-all", test_remove_all);
+  g_test_add_func ("/hash/find", test_find);
+  g_test_add_func ("/hash/foreach", test_foreach);
 
   /* tests for individual bugs */
   g_test_add_func ("/hash/lookup-null-key", test_lookup_null_key);