On Tue, Feb 24, 2015 at 04:12:16PM +0100, Olivier Fourdan wrote: > https://bugs.freedesktop.org/show_bug.cgi?id=89296 > > Signed-off-by: Olivier Fourdan <[email protected]> > --- > > v2: Move the test to LibinputSetProperty() and do it only once > > src/libinput.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/libinput.c b/src/libinput.c > index 9613fbd..ac14adb 100644 > --- a/src/libinput.c > +++ b/src/libinput.c > @@ -1550,8 +1550,14 @@ static int > LibinputSetProperty(DeviceIntPtr dev, Atom atom, XIPropertyValuePtr val, > BOOL checkonly) > { > + InputInfoPtr pInfo = dev->public.devicePrivate; > + struct xf86libinput *driver_data = pInfo->private; > + struct libinput_device *device = driver_data->device; > int rc; > > + if (device == NULL) > + return XI_BadDevice; > + > if (atom == prop_tap) > rc = LibinputSetPropertyTap(dev, atom, val, checkonly); > else if (atom == prop_calibration) > -- > 2.1.0
this turned out to be a bigger problem than expected, it happens whenever a property is set on a disabled device. It's quite easy to reproduce: xinput disable <device name> xinput set-prop <device name> "libinput Left Handed Enabled" 1 I suspect what's happening in XFCE is that you immediately set all properties on all devices without waiting for them to be enabled first. Historically, that didn't matter because drivers never had to worry about this. In the libinput driver this matters because we need libinput to refresh the FDs to check what properties we have. So the patch above is needed but we now run into the issue that this case is not defined in the protocol. I think BadDevice sends the wrong message, the device is valid after all. BadValue likewise, so I'm going with BadMatch. Either way, this can't be fixed in the driver, so we need the clients to work around this and only set properties once the device was enabled. Updated patch: From 97857beac4621f79c6b853c37357573ae11038f7 Mon Sep 17 00:00:00 2001 From: Olivier Fourdan <[email protected]> Date: Tue, 24 Feb 2015 16:12:16 +0100 Subject: [PATCH xf86-input-libinput] Ignore property changes if the device is disabled If the device is present but disabled, the server will still call into SetProperty. We don't have a libinput device to back it up in this case, causing a null-pointer dereference. This is a bug specific to this driver that cannot easily be fixed. All other drivers can handle property changes even if no device is present, here we rely on libinput to make the final call. But without a device path/fd we don't have a libinput reference The protocol doesn't mention this case, so let's pick BadMatch as the least wrong error code. And put a warning in the log, this needs a workaround in the client. Also, if we get here and the device is on, then that's definitely a bug, warn about that. https://bugs.freedesktop.org/show_bug.cgi?id=89296 Signed-off-by: Olivier Fourdan <[email protected]> Signed-off-by: Peter Hutterer <[email protected]> --- src/libinput.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libinput.c b/src/libinput.c index eee3bfb..482e0d7 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1550,8 +1550,20 @@ static int LibinputSetProperty(DeviceIntPtr dev, Atom atom, XIPropertyValuePtr val, BOOL checkonly) { + InputInfoPtr pInfo = dev->public.devicePrivate; + struct xf86libinput *driver_data = pInfo->private; + struct libinput_device *device = driver_data->device; int rc; + if (device == NULL && checkonly) { + BUG_WARN(dev->public.on); + xf86IDrvMsg(pInfo, X_INFO, + "SetProperty on %d called but device is disabled.\n" + "This driver cannot change properties on a disabled device\n", + atom); + return BadMatch; + } + if (atom == prop_tap) rc = LibinputSetPropertyTap(dev, atom, val, checkonly); else if (atom == prop_calibration) -- 2.1.0 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
