After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723316 - g_hash_table_iter_remove() should be explicit whether or not it is safe while iterating the table
g_hash_table_iter_remove() should be explicit whether or not it is safe while...
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: docs
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-01-30 21:27 UTC by Xavier Claessens
Modified: 2014-02-21 20:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GHashTable: Explicitly document that _iter_remove() is safe while iterating (919 bytes, patch)
2014-02-21 20:37 UTC, Xavier Claessens
none Details | Review

Description Xavier Claessens 2014-01-30 21:27:06 UTC
One of the most common crasher I've seen is removing elements of a data structure while iterating it. Typically:

for (l = list; l != NULL; l = l->next)
  {
    if (cond)
      list = g_list_delete_link (list, l);
  }

or:

g_hash_table_foreach (table, {if (cond) g_hash_table_remove (table, key);}, NULL);

or:

while (g_hash_table_iter_next (&iter, &k, &v))
  {
    if (cond)
      g_hash_table_remove (table, k);
  }

Since it is really common mistake I think the doc of g_hash_table_iter_remove() should be explicit about that case. I _think_ it is safe to this, but I'm not entirely sure:

while (g_hash_table_iter_next (&iter, &k, &v))
  {
    if (cond)
      g_hash_table_iter_remove (&iter);
  }
Comment 1 Xavier Claessens 2014-01-30 21:31:37 UTC
While I'm at it, I'm also unsure about:

for (iter = g_sequence_get_begin_iter (seq);
     !g_sequence_iter_is_end (iter);
     iter = g_sequence_iter_next (iter))
  {
    if (cond)
      g_sequence_remove (iter);
  }

The whole GSequence API is weird IMHO, but that's another topic :P
Comment 2 Olivier Brunel (jjacky) 2014-01-30 22:01:15 UTC
I don't know about GSequence, but you're right about the hashtable, calling g_hash_table_iter_remove() like so is safe.
Comment 3 Xavier Claessens 2014-02-21 20:37:26 UTC
Created attachment 269950 [details] [review]
GHashTable: Explicitly document that _iter_remove() is safe while iterating
Comment 4 Xavier Claessens 2014-02-21 20:40:22 UTC
Company approved on IRC. Pushed into master.