Re: [Linuxwacom-devel] [PATCH] Fix button assignment bug introduced in 22bc3028
On Sun, Mar 13, 2011 at 11:00 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Fri, Mar 11, 2011 at 04:02:56PM -0800, Jason Gerecke wrote: I just noticed I was unable to map button presses to keys and after some investigation (yay git bisect!) found the following commit caused the bug: 22bc3028effbdc79d426c0b3dcf586734d4c7532 It appears the original comment accompaning the argc check was wrong. The original 'if' statement was if (argc 0) which would imply XChangeDeviceProperty was to be called if we were *setting* a property, not *unsetting*. This patch should probably be checked more thoroughly since I haven't actually tried to figure out how this function works. At first glance, it seems like the first nitems hunk of a6b9416148e5423c5a0c2632d88dbee5589615 I don't seem to have this ref in my tree. is this one that hasn't been applied yet? Patch looks good though, but I do wonder where this sha came from. Cheers, Peter Looks like I somehow missed the first two hex digits in the SHA. Should be eba6b9416148e5423c5a0c2632d88dbee5589615. The commit subject was xsetwacom: fix button action unsetting. Regardless, after spending far too much time figuring out this function I think this actually needs to be a patch set... What we want to happen is: 1. If the parse fails, leave the function without applying any changes. 2. If we parsed something, call XChangeDeviceProperty for 'param' to apply the necessary side-effects. If we parsed nothing, restore the default and call XDeleteDeviceProperty to remove 'param'. 3. Call XChangeDeviceProperty for 'btnact_prop' to update the master list. Simply going with !unset_prop or nitems 0 (or any boolean combination of the two) will get button assignments working again, but leaves a small hole in #2. I'm going to withdraw this patch for now -- going with nitems 0 is actually more in line with what we need in the end. Jason --- Day xee-nee-svsh duu-'ushtlh-ts'it; nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it. Huu-chan xuu naa~-gha. -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
[Linuxwacom-devel] [PATCH] Fix button assignment bug introduced in 22bc3028
I just noticed I was unable to map button presses to keys and after some investigation (yay git bisect!) found the following commit caused the bug: 22bc3028effbdc79d426c0b3dcf586734d4c7532 It appears the original comment accompaning the argc check was wrong. The original 'if' statement was if (argc 0) which would imply XChangeDeviceProperty was to be called if we were *setting* a property, not *unsetting*. This patch should probably be checked more thoroughly since I haven't actually tried to figure out how this function works. At first glance, it seems like the first nitems hunk of a6b9416148e5423c5a0c2632d88dbee5589615 was actually on the right track... Signed-off-by: Jason Gerecke killert...@gmail.com --- tools/xsetwacom.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c index 3435389..1d7265d 100644 --- a/tools/xsetwacom.c +++ b/tools/xsetwacom.c @@ -1194,7 +1194,7 @@ static void special_map_property(Display *dpy, XDevice *dev, Atom btnact_prop, i fprintf(stderr, Cannot parse keyword '%s'\n, words[i]); } - if (unset_prop) + if (!unset_prop) XChangeDeviceProperty(dpy, dev, prop, XA_INTEGER, 32, PropModeReplace, (unsigned char*)data, nitems); -- 1.7.1 -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH] Fix button assignment bug introduced in 22bc3028
I just acked a similar patch from Peter because his addition of nitems looked needed and I didn't have time to think in depth about earlier patch that caused issue. - if (unset_prop) + if (unset_prop || nitems 0) XChangeDeviceProperty(dpy, dev, prop, XA_INTEGER, 32, PropModeReplace, (unsigned char*)data, nitems); After see'ing yours, I'm thinking maybe yours is right approach although I still not time to think much on it. When you want to unset_prop, its a different function call to delete the property (if I recall correctly). So XChangeDeviceProperty() should be based on !unset_prop. Peter, hope you have time to investigate. I kindly withdraw my ack on other patch and let you and Jason figure it out. Chris On Fri, Mar 11, 2011 at 6:02 PM, Jason Gerecke killert...@gmail.com wrote: I just noticed I was unable to map button presses to keys and after some investigation (yay git bisect!) found the following commit caused the bug: 22bc3028effbdc79d426c0b3dcf586734d4c7532 It appears the original comment accompaning the argc check was wrong. The original 'if' statement was if (argc 0) which would imply XChangeDeviceProperty was to be called if we were *setting* a property, not *unsetting*. This patch should probably be checked more thoroughly since I haven't actually tried to figure out how this function works. At first glance, it seems like the first nitems hunk of a6b9416148e5423c5a0c2632d88dbee5589615 was actually on the right track... Signed-off-by: Jason Gerecke killert...@gmail.com --- tools/xsetwacom.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c index 3435389..1d7265d 100644 --- a/tools/xsetwacom.c +++ b/tools/xsetwacom.c @@ -1194,7 +1194,7 @@ static void special_map_property(Display *dpy, XDevice *dev, Atom btnact_prop, i fprintf(stderr, Cannot parse keyword '%s'\n, words[i]); } - if (unset_prop) + if (!unset_prop) XChangeDeviceProperty(dpy, dev, prop, XA_INTEGER, 32, PropModeReplace, (unsigned char*)data, nitems); -- 1.7.1 -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel -- Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
