[PATCH] dix: don't emulate scroll events for non-existing axes (#47281)

[Peter Hutterer]

On Mon, Apr 30, 2012 at 10:51:25AM -0700, Chase Douglas wrote:
> On 04/26/2012 11:40 PM, Peter Hutterer wrote:
> > Test case:
> > - create a device with REL_HWHEEL and ABS_X and ABS_Y. evdev 2.7.0 will set
> >   that up as device with 1 relative axis
> > - move pointer to VGA1
> > - xrandr --output VGA1 --off
> > 
> > Warps the pointer to the new spot and calls GPE with the x/y mask bits set.
> > When running through the loop to check for scroll event, this overruns the
> > axes and may try to emulate scroll events based on random garbage in the
> > memory. If that memory contained non-zero for the scroll type but near-zero
> > for the increment field, the server would hang in an infinite loop.
> > 
> > This was the trigger for this suggested, never-merged, patch here:
> > http://patchwork.freedesktop.org/patch/9543/
> > 
> > X.Org Bug 47281 <http://bugs.freedesktop.org/show_bug.cgi?id=47281>
> > 
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  dix/getevents.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/dix/getevents.c b/dix/getevents.c
> > index 61790fc..d4e927d 100644
> > --- a/dix/getevents.c
> > +++ b/dix/getevents.c
> > @@ -1602,6 +1602,9 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type,
> >      /* Now turn the smooth-scrolling axes back into emulated button presses
> >       * for legacy clients, based on the integer delta between before and now */
> >      for (i = 0; i < valuator_mask_size(&mask); i++) {
> > +        if (i >= pDev->valuator->numAxes)
> > +            break;
> > +
> >          if (!valuator_mask_isset(&mask, i))
> >              continue;
> >  
> 
> Why not do this instead:
> 
> for (i = 0;
>      i < valuator_mask_size(&mask) && i < pDev->valuator->numAxes;
>      i++) {

personal style. something that isn't intrinsic to the loop shouldn't be part
of the loop condition, imo. Looping over the number of axes is an exception,
not the rule and having a separate break shows this more explicitly.

> 
> The functionality looks correct either way, so
> 
> Reviewed-by: Chase Douglas <chase.douglas at canonical.com>

thanks

Cheers,
  Peter