GNOME Bugzilla – Bug 778382
gtk_css_static_style_get_default doesn't check for a null setting
Last modified: 2017-09-12 15:06:02 UTC
Created attachment 345296 [details] [review] gtk_css_static_style_get_default doesn't check for a null setting On my platform gnome-session-failed is segfaulting with this trace: Core was generated by `/usr/lib/gnome-session/gnome-session-failed'. Program terminated with signal SIGSEGV, Segmentation fault.
+ Trace 237124
The problem seems to be gtk_css_static_style_get_default() failing to handle the fact that gtk_settings_get_default() can return NULL. In attachment the proposed patch.
The only reason why gtk_settings_get_default() returns NULL is that there's no default GdkDisplay, which means there's no display connection, which means there really are no settings. We don't generally check the return value of gtk_settings_get_default(). The only reason why gtk_settings_get_default() even can return NULL is because a bunch of old code ported from GTK+ 2.x has the tendency to call gtk_settings_get_default() without initializing GTK, but if you get this far into the style system then you must have called gtk_init() and have a default GdkDisplay, which means having a default GtkSettings instance.
(In reply to Emmanuele Bassi (:ebassi) from comment #1) > The only reason why gtk_settings_get_default() returns NULL is that there's > no default GdkDisplay, which means there's no display connection, which > means there really are no settings. We don't generally check the return > value of gtk_settings_get_default(). > > The only reason why gtk_settings_get_default() even can return NULL is > because a bunch of old code ported from GTK+ 2.x has the tendency to call > gtk_settings_get_default() without initializing GTK, but if you get this far > into the style system then you must have called gtk_init() and have a > default GdkDisplay, which means having a default GtkSettings instance. Perhaps the documentation for gdk_settings_get_default() could be improved to mention this? Should perhaps also add some assertions or critical warnings on each call to gdk_settings_get_default() in bits of GTK+ which really should have a display to work properly.
The documentation for `gtk_settings_get_default()` already says: ``` * Returns: (nullable) (transfer none): a #GtkSettings object. If there is * no default screen, then returns %NULL. ``` But it would be okay by me to add a g_debug() note that says that you attempted to get the default settings singleton without a GdkDisplay. I'm more dubious about adding assertions because that bit us in the past with applications calling GTK API that "was safe to call in a previous version" without calling gtk_init().
Created attachment 345352 [details] [review] gtksettings: Add a debug message if there is no default GtkSettings Make it slightly more obvious when things are about to slide sideways because a NULL GtkSettings has been returned to a caller. This is a valid return value, but is rarely handled correctly.
Created attachment 345368 [details] [review] gtksettings: Add a debug message if there is no default GtkSettings Make it slightly more obvious when things are about to slide sideways because a NULL GtkSettings has been returned to a caller. This is a valid return value, but is rarely handled correctly.
Is a g_debug here really useful? Even if you compile with --enable-debug, g_debug messages only appear if you also use G_MESSAGES_DEBUG=Gtk.
(In reply to Timm Bäder from comment #6) > Is a g_debug here really useful? Even if you compile with --enable-debug, > g_debug messages only appear if you also use G_MESSAGES_DEBUG=Gtk. …which is what I would do if I were debugging a problem. Up to you.
*** Bug 783171 has been marked as a duplicate of this bug. ***
Review of attachment 345368 [details] [review]: Reviewed by Timm on IRC: “Meh, don't really care. You can push that”.
Attachment 345368 [details] pushed as 80b10aa - gtksettings: Add a debug message if there is no default GtkSettings