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! -- Chase _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
