GNOME Bugzilla – Bug 679736
Using [Ctrl]+[Shift]+[+] as shortcut in Wacom panel crashes gnome-settings-daemon
Last modified: 2012-07-11 11:41:57 UTC
Defining a shortcut as [Ctrl]+[Shift]+[+] in gnome-control-center for a pad button and pressing the pad button will crash gnome-settings-daemon with a BadValue XError. (gnome-settings-daemon:23040): wacom-plugin-DEBUG: Received event button press 'buttonB' ('2') on device 'Wacom Intuos5 touch M' ('16') (gnome-settings-daemon:23040): wacom-plugin-DEBUG: Emitting '<Primary><Shift>plus' (keyval: 43, keycode: 0 mods: 0x5) (gnome-settings-daemon:23040): Gdk-ERROR **: The program 'gnome-settings-daemon' received an X Window System error. This probably reflects a bug in the program. The error was 'BadValue (integer parameter out of range for operation)'. (Details: serial 1170 error_code 2 request_code 142 minor_code 2) (Note to programmers: normally, X errors are reported asynchronously; that is, you will receive the error a while after causing it. To debug your program, run it with the GDK_SYNCHRONIZE environment variable to change this behavior. You can then get a meaningful backtrace from your debugger if you break on the gdk_x_error() function.) Trace/breakpoint trap Notice the "keycode: 0" in the debug logs, that's what passed to XTestFakeKeyEvent() in generate_key() and that causes a BadValue XError because 0 is not a valid keycode. This is bacause the keycode is not found in the current group so it needs to be looked in group 0, but the code checks for "level > 0" no "group > 0" thus no keycode is found and 0 is passed.
Created attachment 218523 [details] [review] Proposed patch Patch tofix the issue. Before: gnome-settings-daemon:22325): wacom-plugin-DEBUG: Received event button press 'buttonB' ('2') on device 'Wacom Intuos5 touch M' ('16') (gnome-settings-daemon:22325): wacom-plugin-DEBUG: Emitting '<Primary><Shift>plus' (keyval: 43, keycode: 0 mods: 0x5) => BadValue on keycode After: (gnome-settings-daemon:2056): wacom-plugin-DEBUG: Received event button press 'buttonB' ('2') on device 'Wacom Intuos5 touch M' ('16') (gnome-settings-daemon:2056): wacom-plugin-DEBUG: Emitting '<Primary><Shift>plus' (keyval: 43, keycode: 21 mods: 0x5) Event is generated
Review of attachment 218523 [details] [review]: Need a better explanation than that.
What part exactly needs better explanation? The comment says: /* Couldn't find it in the current group? Look in group 0 */ The code does: for (i = 0; i < n_keys; i++) { if (keys[i].level > 0) continue; keycode = keys[i].keycode; } Checking for "level > 0" no keycode is found and keycode remains 0 and that is not a valid keycode to pass to XTestFakeKeyEvent() Using "group" fixes that so I think you meant "group" here, not "level" based on your comment and the code.
(In reply to comment #3) > What part exactly needs better explanation? The commit log message. "Because that makes it work" isn't a _reason_ for a change, it's the result of a change.
Created attachment 218534 [details] [review] Updated patch (In reply to comment #4) > (In reply to comment #3) > > What part exactly needs better explanation? > > The commit log message. "Because that makes it work" isn't a _reason_ for a > change, it's the result of a change. Humm, I still don't get what you mean here, the commit log doesn't read "because that makes it work", it reads "Use field group not level in keymaps otherwise some keycode won't be found" which is the reason for the change I believe. It now reads "use keymap group to search group 0 in case the keycode isn't found in current group", would that be better?
Created attachment 218538 [details] [review] wacom: Fix crash for shortcuts requiring Shift As per code comment, we want to ignore non-zero groups, not non-zero levels, otherwise we wouldn't be able to find keycodes for keysyms that require Shift. Fixes crasher pressing a button with "Ctrl+Alt++" associated to it.
Created attachment 218540 [details] [review] wacom: Fix crash for shortcuts requiring Shift As per code comment, we want to ignore non-zero groups, not non-zero levels, otherwise we wouldn't be able to find keycodes for keysyms that require Shift. Fixes crasher pressing a button with "Ctrl+Alt++" associated to it.
Created attachment 218541 [details] [review] wacom: Never try to create fake events with 0 keycode XTestFakeKeyEvent() doesn't like it.
Created attachment 218542 [details] [review] wacom: Avoid X errors when calling XTestFakeKeyEvent()
Attachment 218540 [details] pushed as 7a55cc7 - wacom: Fix crash for shortcuts requiring Shift Attachment 218541 [details] pushed as 09ad656 - wacom: Never try to create fake events with 0 keycode Attachment 218542 [details] pushed as 67f96d4 - wacom: Avoid X errors when calling XTestFakeKeyEvent()