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 661286 - [PATCH] Remove N_GTK_STATES assertion in ui/theme.c
[PATCH] Remove N_GTK_STATES assertion in ui/theme.c
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
3.2.x
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2011-10-08 20:03 UTC by Sandro Mani
Modified: 2011-10-08 21:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.05 KB, text/plain)
2011-10-08 20:03 UTC, Sandro Mani
  Details
Patch (1.23 KB, patch)
2011-10-08 20:27 UTC, Sandro Mani
reviewed Details | Review
Patch v2, detailed commit message (1.48 KB, patch)
2011-10-08 21:21 UTC, Sandro Mani
committed Details | Review

Description Sandro Mani 2011-10-08 20:03:31 UTC
Using mutter-3.2.0-1.fc16.x86_64

The assertion at theme.c::1340, i.e.

g_assert (spec->data.gtk.state < N_GTK_STATES);

is a leftover from when Mutter used GtkStateFlags. Mutter has since been modified to use GtkStateType, but the assertion was not removed, which causes some themes to crash mutter with assertion failed (i.e. Nodoka).
The reason is that before, the GTK_STATE_* went from 0 to 4 (and N_GTK_STATES is 5), while now GTK_STATE_FLAG_* have values 0, 1, 2, 4, 8, .. 32.

I suggest to remove the assertion completely, since an invalid state specification is already handled at theme.c::1317 anyway.

A corresponding patch is attached.
Comment 1 Sandro Mani 2011-10-08 20:03:59 UTC
Created attachment 198625 [details]
Patch
Comment 2 Jasper St. Pierre (not reading bugmail) 2011-10-08 20:10:59 UTC
Can you attach a patch created by the output of git format-patch?
Comment 3 Sandro Mani 2011-10-08 20:27:26 UTC
Created attachment 198627 [details] [review]
Patch

patch from git format-patch
Comment 4 Jasper St. Pierre (not reading bugmail) 2011-10-08 20:34:43 UTC
(In reply to comment #0)
> Using mutter-3.2.0-1.fc16.x86_64
> 
> The assertion at theme.c::1340, i.e.
> 
> g_assert (spec->data.gtk.state < N_GTK_STATES);
> 
> is a leftover from when Mutter used GtkStateFlags. Mutter has since been
> modified to use GtkStateType, but the assertion was not removed, which causes
> some themes to crash mutter with assertion failed (i.e. Nodoka).
> The reason is that before, the GTK_STATE_* went from 0 to 4 (and N_GTK_STATES
> is 5), while now GTK_STATE_FLAG_* have values 0, 1, 2, 4, 8, .. 32.

It seems like mutter still uses GtkStateFlags

http://git.gnome.org/browse/mutter/tree/src/ui/theme-private.h#n302

Maybe you have a distro patch?
Comment 5 Sandro Mani 2011-10-08 20:40:10 UTC
Darn, I meant that it now uses GtkStateFlags and _used to use_ GtkStateType. Sorry... (but patch is still valid).
Comment 6 Jasper St. Pierre (not reading bugmail) 2011-10-08 21:04:14 UTC
Review of attachment 198627 [details] [review]:

The patch looks fine to me. Add a proper commit message like:

    theme: Remove outdated assertion

    The theme state used to use GtkStateType, but was ported over to GtkStateFlags,
    leaving behind a broken assertion that fails when [put some examples of when
    you've hit the assertion].

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

and it should be good to go.
Comment 7 Sandro Mani 2011-10-08 21:21:03 UTC
Created attachment 198634 [details] [review]
Patch v2, detailed commit message
Comment 8 Jasper St. Pierre (not reading bugmail) 2011-10-08 21:31:05 UTC
Pushed with a few commit message fixups (79-char limit, remove "See "). Thanks!