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.hutte...@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 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel