Fix g_hash_table_iter_replace

When reusing an existing key 'internally', we must avoid calling
the key_destroy function on the old key.

https://bugzilla.gnome.org/show_bug.cgi?id=662544

glib/ghash.c

@@ -839,10 +839,16 @@ g_hash_table_iter_remove (GHashTableIter *iter)
  * @hash_table: our #GHashTable
  * @node_index: pointer to node to insert/replace
  * @key_hash: key hash
- * @key: key to replace with
+ * @key: key to replace with, or %NULL
  * @value: value to replace with
+ * @keep_new_key: whether to replace the key in the node with @key
+ * @reusing_key: whether @key was taken out of the existing node
  *
  * Inserts a value at @node_index in the hash table and updates it.
+ *
+ * If @key has been taken out of the existing node (ie it is not
+ * passed in via a g_hash_table_insert/replace) call, then @reusing_key
+ * should be %TRUE.
  */
 static void
 g_hash_table_insert_node (GHashTable *hash_table,
@@ -850,7 +856,8 @@ g_hash_table_insert_node (GHashTable *hash_table,
                           guint       key_hash,
                           gpointer    key,
                           gpointer    value,
-                          gboolean    keep_new_key)
+                          gboolean    keep_new_key,
+                          gboolean    reusing_key)
 {
   guint old_hash;
   gpointer old_key;
@@ -891,7 +898,7 @@ g_hash_table_insert_node (GHashTable *hash_table,
 
   if (HASH_IS_REAL (old_hash))
     {
-      if (hash_table->key_destroy_func)
+      if (hash_table->key_destroy_func && !reusing_key)
         hash_table->key_destroy_func (keep_new_key ? old_key : key);
       if (hash_table->value_destroy_func)
         hash_table->value_destroy_func (old_value);
@@ -932,7 +939,7 @@ g_hash_table_iter_replace (GHashTableIter *iter,
   node_hash = ri->hash_table->hashes[ri->position];
   key = ri->hash_table->keys[ri->position];
 
-  g_hash_table_insert_node (ri->hash_table, ri->position, node_hash, key, value, TRUE);
+  g_hash_table_insert_node (ri->hash_table, ri->position, node_hash, key, value, TRUE, TRUE);
 
 #ifndef G_DISABLE_ASSERT
   ri->version++;
@@ -1127,7 +1134,7 @@ g_hash_table_insert_internal (GHashTable *hash_table,
 
   node_index = g_hash_table_lookup_node (hash_table, key, &key_hash);
 
-  g_hash_table_insert_node (hash_table, node_index, key_hash, key, value, keep_new_key);
+  g_hash_table_insert_node (hash_table, node_index, key_hash, key, value, keep_new_key, FALSE);
 }
 
 /**

glib/tests/hash.c

@@ -1098,6 +1098,52 @@ test_internal_consistency (void)
   g_hash_table_unref (h);
 }
 
+static void
+my_key_free (gpointer v)
+{
+  gchar *s = v;
+  g_assert (s[0] != 'x');
+  s[0] = 'x';
+  g_free (v);
+}
+
+static void
+my_value_free (gpointer v)
+{
+  gchar *s = v;
+  g_assert (s[0] != 'y');
+  s[0] = 'y';
+  g_free (v);
+}
+
+static void
+test_iter_replace (void)
+{
+  GHashTable *h;
+  GHashTableIter iter;
+  gpointer k, v;
+  gchar *s;
+
+  g_test_bug ("662544");
+
+  h = g_hash_table_new_full (g_str_hash, g_str_equal, my_key_free, my_value_free);
+
+  g_hash_table_insert (h, g_strdup ("A"), g_strdup ("a"));
+  g_hash_table_insert (h, g_strdup ("B"), g_strdup ("b"));
+  g_hash_table_insert (h, g_strdup ("C"), g_strdup ("c"));
+
+  g_hash_table_iter_init (&iter, h);
+
+  while (g_hash_table_iter_next (&iter, &k, &v))
+    {
+       s = (gchar*)v;
+       g_assert (g_ascii_islower (s[0]));
+       g_hash_table_iter_replace (&iter, g_strdup (k));
+    }
+
+  g_hash_table_unref (h);
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -1123,6 +1169,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/hash/lookup-null-key", test_lookup_null_key);
   g_test_add_func ("/hash/destroy-modify", test_destroy_modify);
   g_test_add_func ("/hash/consistency", test_internal_consistency);
+  g_test_add_func ("/hash/iter-replace", test_iter_replace);
 
   return g_test_run ();