On Wed, Feb 04, 2015 at 04:45:37PM -0500, Olivier Fourdan wrote: > Hi Peter, > > Just to clarify, evdev_accel_config_set_speed() calls filter_set_speed() > which calls accelerator_set_speed() which reaches the assert(). > > My patch basically removes the assert() and replaces it by a return false > so that it fails gracefully.
yeah, I understand that bit but I don't know is how you get here. the xorg libinput driver calls libinput_device_config_accel_set_speed() which has the range check. This should be the only entry point for evdev_accel_config_set_speed() so really, this assert should never trigger. I tried triggering this through xinput set-prop and it seems to work as expected with the random values I tried. Can you attach gdb to see the callstack of how you get there? Cheers, Peter > ----- Original Message ----- > From: "Olivier Fourdan" <[email protected]> > To: "Peter Hutterer" <[email protected]> > Cc: [email protected] > Sent: Wednesday, 4 February, 2015 10:28:22 PM > Subject: Re: [PATCH libinput] Do not abort on invalid speed. > > Hi Peter, > > Weird, that occurs with xf86-drv-libinput and libinput both from git as of > today. > > I don't think this is an uninitialized variable, it's really the Xorg client > (read, my code) that sends values out of range and that takes the whole X > server down because of the assert()... > > Cheers, > Olivier > > ----- Original Message ----- > From: "Peter Hutterer" <[email protected]> > To: "Olivier Fourdan" <[email protected]> > Cc: [email protected] > Sent: Wednesday, 4 February, 2015 10:15:56 PM > Subject: Re: [PATCH libinput] Do not abort on invalid speed. > > On Wed, Feb 04, 2015 at 10:34:25AM +0100, Olivier Fourdan wrote: > > Libinput's accelerator_set_speed() asserts the given speed value is > > within the [-1.0, 1.0] range. > > > > When using xf86-input-libinput, a client may try to set an invalid speed > > which will cause the entire Xserver to abort. > > > > Instead of aborting on invalid speed values, simply return a failure > > (and log a message). > > > > Signed-off-by: Olivier Fourdan <[email protected]> > > I think this was fixed with b21fbfd947d4bc18b766c46ab1d85a3f8d0977f4 and > 199cd87b071f6fd0d4a1065ae3d678552259f883, it was caused by an uninitialized > variable. > https://bugs.freedesktop.org/show_bug.cgi?id=88899 > > libinput_device_config_accel_set_speed() already has the checks for [-1, 1], > so by the time we get here a speed outside those boundaries is a bug. > > Cheers, > Peter > > > --- > > src/evdev.c | 5 ++++- > > src/filter.c | 3 ++- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/src/evdev.c b/src/evdev.c > > index 6e318dc..c96a66b 100644 > > --- a/src/evdev.c > > +++ b/src/evdev.c > > @@ -1194,8 +1194,11 @@ evdev_accel_config_set_speed(struct libinput_device > > *device, double speed) > > { > > struct evdev_device *dev = (struct evdev_device *)device; > > > > - if (!filter_set_speed(dev->pointer.filter, speed)) > > + if (!filter_set_speed(dev->pointer.filter, speed)) { > > + log_bug_libinput(device->seat->libinput, > > + "Invalid speed %f\n", speed); > > return LIBINPUT_CONFIG_STATUS_INVALID; > > + } > > > > return LIBINPUT_CONFIG_STATUS_SUCCESS; > > } > > diff --git a/src/filter.c b/src/filter.c > > index 72ef760..91afdcd 100644 > > --- a/src/filter.c > > +++ b/src/filter.c > > @@ -260,7 +260,8 @@ accelerator_set_speed(struct motion_filter *filter, > > struct pointer_accelerator *accel_filter = > > (struct pointer_accelerator *)filter; > > > > - assert(speed >= -1.0 && speed <= 1.0); > > + if (speed < -1.0 || speed > 1.0) > > + return false; > > > > /* delay when accel kicks in */ > > accel_filter->threshold = DEFAULT_THRESHOLD - speed/6.0; > > -- > > 2.1.0 > > _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
