On Tue, Mar 22, 2011 at 10:47:02AM -0700, Jason Gerecke wrote: > On Mon, Mar 21, 2011 at 9:11 PM, Peter Hutterer > <[email protected]> wrote: > > On Fri, Mar 18, 2011 at 06:01:16PM -0700, Jason Gerecke wrote: > >> Crash first appears in ab311bf20535acd6e7201e024bc311e0e15b5b6b. > >> This commit rewrote wcmUpdateButtonKeyActions with one-indexed > >> arrays in mind and extended the 'keys' array in _WacomDeviceRec > >> to make room. However, the 'wheel_keys' and 'strip_keys' were > >> not updated. > >> > >> Running e.g. `xsetwacom --set pad StripLeftDown button 4` results > >> in a call to wcmUpdateButtonKeyActions. Its call to memset oversteps > >> the array bounds and just happens to zero the 'common' pointer. The > >> next tablet event results in a segfault as the driver tries to > >> dereference the pointer. > > > > doh. i think some audit in the driver is needed to switch to either > > one-indexed or zero-indexed buttons everywhere. right now, we seem to have > > both mixed and that's rather fatal. > > > > That would probably be good a good idea. Any idea why one-indexing was > switched to? I guess it'd make writing user interface code easier (you > just index into the button array at the number they give), but > violates the principle of least astonishment...
probably because I come from the server side, where buttons are always one-indexed. and there are plenty of functions (in the server) where a "int button" parameter exists, but a button may not be passed into. In this case, having zero as "no button" is the common approach. > > are you sure this is the right patch though? wcmCommon.c seems to use the > > right indices (minus the patch I've just CC'd you on). so it looks like > > there's more out of whack than just the array size. > > > > wcmCommon.c might be using the right indicies, or it might not. It's > difficult to say at the moment since strip mapping (and I assume wheel > mapping since they're virtually identical) stopped working back in > August with commit eea34fad. Between fixing that and switching > everything to use zero- or one-indexed arrays, this bit of code needs > some love. > > This patch was more of a stop-gap to fix the crash before I got around > to restoring mapping functionality. Hopefully I can have everything > done before the April 1 freeze (not sure how much code I'll need to > touch, so I'd rather not try to slip it in after...) but I'm not sure > I have the time. ok, I'll try to squeeze some time in to look at it. Cheers, Peter ------------------------------------------------------------------------------ Enable your software for Intel(R) Active Management Technology to meet the growing manageability and security demands of your customers. Businesses are taking advantage of Intel(R) vPro (TM) technology - will your software be a part of the solution? Download the Intel(R) Manageability Checker today! http://p.sf.net/sfu/intel-dev2devmar _______________________________________________ Linuxwacom-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
