GNOME Bugzilla – Bug 621651
gtk_tree_view_set_cursor is not always safe with invalid path
Last modified: 2011-04-16 18:49:06 UTC
I have a GtkTreeView with a GtkTreeModelFilter which makes everything hidden. So my view is empty. The view has a custom function to tell if a row is separator, using gtk_tree_view_set_row_separator_func(). Then I call gtk_tree_view_set_cursor() with path "0:1" which is invalid since all rows are invisible. I get that backtrace: Gtk-CRITICAL **: gtk_tree_model_filter_get_value: assertion `GTK_TREE_MODEL_FILTER (model)->priv->stamp == iter->stamp' failed aborting... Program received signal SIGABRT, Aborted. 0xb7fe2422 in __kernel_vsyscall () (gdb) bt
+ Trace 222418
user_data=0x82faa88) at empathy-live-search.c:154 The problem is that my custom empathy_contact_list_store_row_separator_func() is called with an invalid GtkTreeIter, resulting in a crash later. Looking inside the code of GtkTreeView, I see that the path I give to gtk_tree_view_set_cursor() is passed with no check to row_is_separator(). That function takes an iter for the path and call my custom function without checking first if the iter is valid.
Made a patch that fix this probably. Needs testing though :) http://git.collabora.co.uk/?p=user/xclaesse/gtk.git;a=shortlog;h=refs/heads/bug621651
@Kris: This patch should be pretty easy to review.
Is there a chance to write a small regression test case for this?
Please for the future just attach patches here in bugzilla, I needed three or four extra clicks to get to the patch. On the actual patch: i) The modification in row_is_separator() is unnecessary and is an artifact of the change in gtk_tree_view_set_cursor(). Just get it right in gtk_tree_view_set_cursor() instead. ii) There has been a discussion on whether or not and if yes how functions like this should fail on invalid paths. I cannot remember the exact outcome and need to read up on that. For this I need to find time.
(In reply to comment #4) > i) The modification in row_is_separator() is unnecessary and is an artifact of > the change in gtk_tree_view_set_cursor(). Just get it right in > gtk_tree_view_set_cursor() instead. It is just sanity check, but that can be dropped indeed. > ii) There has been a discussion on whether or not and if yes how functions > like this should fail on invalid paths. I cannot remember the exact outcome > and need to read up on that. For this I need to find time. The doc of that func explicitely says it is fine to have invalid path. I don't think we should change this, at least not in GTK2.x.
Seems to have been fixed already in commit 405b54c72e35dba0ec47c28f123af3438969f6df.