[PATCH 2/2] Handle non continuous valuator data in getValuatorEvents
chase.douglas at canonical.com
Mon Mar 28 12:11:37 PDT 2011
On 03/28/2011 03:02 PM, Jamey Sharp wrote:
> Hi Chase! I thoroughly sympathize with removing code duplication,
> especially once it's gotten complicated like this, but please, no
> macro magic...
> After trying a bunch of alternatives with GCC 4.4.5 on x86-64 to see
> whether it would generate sane code, it looks to me like the right
> answer is a getValuatorValue static function implementing the
> right-hand side of the assignment, and leaving the assignment itself
> written out explicitly in the switch.
I'm fine with this. I'll send a second revision out shortly.
> A less important complaint: It wasn't obvious to me on initial review
> that it was safe to remove the assignment "dev = NULL". It turns out
> that it is safe because dixLookupDevice ensures it's NULL on failure,
> and the loop only runs if num_valuators > 0, in which case
> dixLookupDevice has certainly been called first. But perhaps some
> short comment is in order?
Actually, this was just sloppy coding on my part :). I think it's best
to leave it initialized to NULL as it was before. Thanks for catching this!
More information about the xorg-devel