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

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);
@@ -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,9 +1378,9 @@ 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
  **/
@@ -1376,18 +1390,30 @@ g_hash_table_find (GHashTable      *hash_table,
                    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;
     }
 

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);