[PATCH libinput] evdev: drop relative x/y motion from a device not marked as pointer

[Peter Hutterer]

On Tue, Aug 18, 2015 at 06:06:56PM -0700, Bill Spitzak wrote:
> On Mon, Aug 17, 2015 at 10:08 PM, Peter Hutterer <peter.hutterer at who-t.net>
> wrote:
> 
> > A device with REL_X/Y and keys gets marked only as ID_INPUT_KEY,
> > initializes
> > as keyboard and then segfaults when we send x/y coordinates - pointer
> > acceleration never initializes.
> >
> > Ignore the events and log a bug instead. This intentionally only papers
> > over
> > the underlying issue, let's wait for a real device to trigger this and then
> > look at the correct solution.
> >
> > +static inline bool
> > +evdev_reject_relative(struct evdev_device *device,
> > +                     const struct input_event *e,
> > +                     uint64_t time)
> > +{
> > +       struct libinput *libinput = device->base.seat->libinput;
> > +
> > +       if ((e->code == REL_X || e->code == REL_Y) &&
> > +           (device->seat_caps & EVDEV_DEVICE_POINTER) == 0) {
> > +               switch (ratelimit_test(&device->nonpointer_rel_limit)) {
> > +               case RATELIMIT_PASS:
> > +                       log_bug_libinput(libinput,
> > +                                        "REL_X/Y from device '%s', but
> > this device is not a pointer\n",
> > +                                        device->devname);
> > +                       break;
> > +               case RATELIMIT_THRESHOLD:
> > +                       log_bug_libinput(libinput,
> > +                                        "REL_X/Y event flood from '%s'\n",
> > +                                        device->devname);
> > +                       break;
> > +               case RATELIMIT_EXCEEDED:
> > +                       break;
> > +               }
> > +
> > +               return true;
> > +       }
> > +
> > +       return false;
> > +}
> >
> 
> It seems like some kind of ratelimit_log(&limit, "format", ...) function
> might be useful so this is not copied everywhere.

good idea, patch is in flight.

> Also wondering if there is a direct test for "pointer acceleration never
> initializes" and perhaps you should do that test instead.

not the same, we can concievably have a device that is a pointer but does
not have pointer acceleration. So checking for the seat caps here is better,
even though the crash is casued by the accel filters missing.
 
> > +
> >  static inline void
> >  evdev_process_relative(struct evdev_device *device,
> >                        struct input_event *e, uint64_t time)
> > @@ -645,6 +675,9 @@ evdev_process_relative(struct evdev_device *device,
> >         struct normalized_coords wheel_degrees = { 0.0, 0.0 };
> >         struct discrete_coords discrete = { 0.0, 0.0 };
> >
> > +       if (evdev_reject_relative(device, e, time))
> > +               return;
> > +
> >         switch (e->code) {
> >         case REL_X:
> >                 if (device->pending_event != EVDEV_RELATIVE_MOTION)
> >
> 
> I think it would look better and be clearer if you put the
> evdev_reject_relative call in the REL_X and REL_Y cases of the switch (you
> could also remove the test for REL_X/Y from the function itself).

it's a generic function even though it only handles one case at this
point, see evdev_reject_device() which is similar. The reason is simple:
mental capacity is limited, this approach separates the "when do we reject
events" from "how do we handle events", you only need to care about one at a
time.

Cheers,
   Peter