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 682080 - Gtk:ERROR:gtktoolbar.c:2271:logical_to_physical: assertion failed: (logical == 0)
Gtk:ERROR:gtktoolbar.c:2271:logical_to_physical: assertion failed: (logical =...
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkToolbar
3.20.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2012-08-17 09:56 UTC by Christophe Fergeau
Modified: 2016-05-19 12:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
toolbar: Use g_assert_cmpint instead of just g_assert (1.01 KB, patch)
2016-05-10 17:18 UTC, Timm Bäder
rejected Details | Review
toolbar: Limit item position to number of contained elements (803 bytes, patch)
2016-05-10 17:18 UTC, Timm Bäder
accepted-commit_now Details | Review

Description Christophe Fergeau 2012-08-17 09:56:12 UTC
This small testcase triggers this assertion in gtk+:

#include <gtk/gtk.h>

void main (int argc, char **argv)
{
    GtkWidget *toolbar;
    GtkToolItem *button;

    gtk_init (&argc, &argv);

    toolbar = gtk_toolbar_new ();
    button = gtk_toggle_tool_button_new ();

    gtk_toolbar_insert (GTK_TOOLBAR (toolbar), button, 1);
}


It's coming from:

static gint
logical_to_physical (GtkToolbar *toolbar,
                     gint        logical)
{
  GtkToolbarPrivate *priv = toolbar->priv;
  GList *list;
  gint physical;

  g_assert (logical >= 0);

  physical = 0;
  for (list = priv->content; list; list = list->next)
    {
      ToolbarContent *content = list->data;

      if (!toolbar_content_is_placeholder (content))
        {
          if (logical == 0)
            break;
          logical--;
        }

      physical++;
    }

  g_assert (logical == 0);

  return physical;
}

This function asserts if the toolbar contains less 'logical' elements. The test case is trying to insert an item at position 1 in an empty toolbar, so the assert is triggered. Maybe a g_return_val_if_fail (logical == 0, physical); would be better than the assert?
Comment 1 Christophe Fergeau 2016-05-10 08:12:47 UTC
Still occurring with latest stable gtk+ with the same test case, setting 'version' to 3.20.x.
Comment 2 Timm Bäder 2016-05-10 17:17:04 UTC
I feel like a simple

position = MIN (position, g_list_length (priv->content))

in gtk_toolbar_insert would be the correct thing to do in any case since values greater than that have no meaning anyway. I don't usually use toolbars though, but I'm attaching a patch anyway.
Comment 3 Timm Bäder 2016-05-10 17:18:04 UTC
Created attachment 327604 [details] [review]
toolbar: Use g_assert_cmpint instead of just g_assert
Comment 4 Timm Bäder 2016-05-10 17:18:28 UTC
Created attachment 327605 [details] [review]
toolbar: Limit item position to number of contained elements
Comment 5 Matthias Clasen 2016-05-12 04:05:10 UTC
Review of attachment 327605 [details] [review]:

looks fine to me
Comment 6 Matthias Clasen 2016-05-12 04:05:52 UTC
Review of attachment 327604 [details] [review]:

I don't like mixing fancy-printing-asserts-for-tests with production code.
Comment 7 Timm Bäder 2016-05-12 16:28:10 UTC
Pushed as 63be0deb0f112c354d3b4d2bbee44e6512d24358.