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 621651 - gtk_tree_view_set_cursor is not always safe with invalid path
gtk_tree_view_set_cursor is not always safe with invalid path
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkTreeView
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtktreeview-bugs
gtktreeview-bugs
Depends on:
Blocks: 621629
 
 
Reported: 2010-06-15 14:39 UTC by Xavier Claessens
Modified: 2011-04-16 18:49 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Xavier Claessens 2010-06-15 14:39:51 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
  • #0 __kernel_vsyscall
  • #1 *__GI_raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 64
  • #2 *__GI_abort
    at abort.c line 92
  • #3 g_logv
    from /lib/libglib-2.0.so.0
  • #4 g_log
    from /lib/libglib-2.0.so.0
  • #5 g_return_if_fail_warning
    from /lib/libglib-2.0.so.0
  • #6 gtk_tree_model_filter_get_value
    at /build/buildd/gtk+2.0-2.20.1/gtk/gtktreemodelfilter.c line 2401
  • #7 IA__gtk_tree_model_get_value
    at /build/buildd/gtk+2.0-2.20.1/gtk/gtktreemodel.c line 1147
  • #8 IA__gtk_tree_model_get_valist
    at /build/buildd/gtk+2.0-2.20.1/gtk/gtktreemodel.c line 1444
  • #9 IA__gtk_tree_model_get
    at /build/buildd/gtk+2.0-2.20.1/gtk/gtktreemodel.c line 1408
  • #10 empathy_contact_list_store_row_separator_func
    at empathy-contact-list-store.c line 729
  • #11 row_is_separator
  • #12 gtk_tree_view_real_set_cursor
    at /build/buildd/gtk+2.0-2.20.1/gtk/gtktreeview.c line 12541
  • #13 IA__gtk_tree_view_set_cursor_on_cell
    at /build/buildd/gtk+2.0-2.20.1/gtk/gtktreeview.c line 12706
  • #14 IA__gtk_tree_view_set_cursor
    at /build/buildd/gtk+2.0-2.20.1/gtk/gtktreeview.c line 12652
  • #15 contact_list_view_search_text_notify_cb
    at empathy-contact-list-view.c line 1225
  • #16 g_cclosure_marshal_VOID__PARAM
    from /usr/lib/libgobject-2.0.so.0
  • #17 g_closure_invoke
    from /usr/lib/libgobject-2.0.so.0
  • #18 ??
    from /usr/lib/libgobject-2.0.so.0
  • #19 g_signal_emit_valist
    from /usr/lib/libgobject-2.0.so.0
  • #20 g_signal_emit
    from /usr/lib/libgobject-2.0.so.0
  • #21 ??
    from /usr/lib/libgobject-2.0.so.0
  • #22 ??
    from /usr/lib/libgobject-2.0.so.0
  • #23 g_object_notify
    from /usr/lib/libgobject-2.0.so.0
  • #24 live_search_text_changed
    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.
Comment 1 Xavier Claessens 2010-06-15 15:14:34 UTC
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
Comment 2 Xavier Claessens 2010-06-15 19:52:02 UTC
@Kris: This patch should be pretty easy to review.
Comment 3 Christian Dywan 2010-06-17 11:18:52 UTC
Is there a chance to write a small regression test case for this?
Comment 4 Kristian Rietveld 2010-06-17 11:31:20 UTC
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.
Comment 5 Xavier Claessens 2010-06-17 11:35:21 UTC
(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.
Comment 6 Kristian Rietveld 2011-04-16 18:49:06 UTC
Seems to have been fixed already in commit 405b54c72e35dba0ec47c28f123af3438969f6df.