GNOME Bugzilla – Bug 696707
g-s-d cursor plugin doesn't fail gracefully
Last modified: 2013-04-15 11:27:48 UTC
Hi, Bug #696118 added code to gracefully fail if running on an old xserver. Unfortunately the gnome-3-8 code doesn't pass the GError to gnome-desktop, which means the GError is unset when returning from gsd_cursor_manager_start() and we crash when trying to print a g_warning() with error->message: Program received signal SIGSEGV, Segmentation fault. 0xb36095de in impl_activate (plugin=0x80acb30) at gsd-cursor-plugin.c:29 29 GNOME_SETTINGS_PLUGIN_REGISTER (GsdCursor, gsd_cursor) (gdb) bt at gnome-settings-plugin-info.c:431 at gnome-settings-manager.c:95 func=func@entry=0x804bfa0 <maybe_activate_plugin>, user_data=user_data@entry=0x0) at /tmp/buildd/glib2.0-2.36.0/./glib/gslist.c:896 Note that the same will happen if one of the X checks fail. The attached patches fix this. They are against gnome-3-8, but I think we should include the first and the third on master (the second is not needed on master as gnome_idle_monitor_new_for_device() takes the GError there.
Created attachment 239960 [details] [review] cursor: add error quark
Created attachment 239961 [details] [review] cursor: set an error if we can't create a monitor Otherwise we crash in impl_activate()
Created attachment 239962 [details] [review] cursor: set error if we don't have the necessary X support Otherwise we crash in impl_activate()
I can confirm this fixes an immediate runtime crash of gnome-settings-daemon on OpenBSD. I'd really like this or a variation of it to be pushed... Thanks.
Review of attachment 239960 [details] [review]: No need for this, the error can be the same used in gnome-desktop master.
Review of attachment 239961 [details] [review]: ::: plugins/cursor/gsd-cursor-manager.c @@ +180,3 @@ + if (!monitor) { + g_set_error (error, GSD_CURSOR_MANAGER_ERROR, + GSD_CURSOR_MANAGER_ERROR_FAILED, Use G_IO_ERROR and G_IO_ERROR_NOT_SUPPORTED instead.
Review of attachment 239962 [details] [review]: ::: plugins/cursor/gsd-cursor-manager.c @@ +296,2 @@ if (supports_cursor_xfixes () == FALSE) { + g_set_error (error, GSD_CURSOR_MANAGER_ERROR, Ditto. This needs to go to master as well.
Created attachment 240587 [details] [review] cursor: set an error if we can't create a monitor Otherwise we crash in impl_activate()
Created attachment 240588 [details] [review] cursor: set error if we don't have the necessary X support Otherwise we crash in impl_activate()
OK to commit these two to gnome-3-8 and the last one to master? I'd like to get this in for 3.8.1.
Review of attachment 240587 [details] [review]: ::: plugins/cursor/gsd-cursor-manager.c @@ +171,3 @@ + if (!monitor) { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, + "Per-device idletime monitor not available"); Fine.
Review of attachment 240588 [details] [review]: Looks good.
Comment on attachment 240587 [details] [review] cursor: set an error if we can't create a monitor Pushed to gnome-3-8
Comment on attachment 240588 [details] [review] cursor: set error if we don't have the necessary X support Pushed to gnome-3-8 and master.