[PATCH xserver 3/3] ddx: add new call to purge input devices that weren't added
Hans de Goede
hdegoede at redhat.com
Fri Oct 21 09:08:04 UTC 2016
Hi,
On 20-10-16 23:50, Peter Hutterer wrote:
> Special case for the systemd-logind case in xfree86: when we're vt-switched
> away and a device is plugged in, we get a paused fd from logind. Since we
> can't probe the device or do anything with it, we store that device in the
> xfree86 and handle it later when we vt-switch back. The device is not added to
> inputInfo.devices until that time.
>
> When the device is removed while still vt-switched away, the the config system
> never notifies the DDX. It only runs through inputInfo.devices and our device
> was never added to that.
>
> When a device is plugged in, removed, and plugged in again while vt-switched
> away, we have two entries in the xfree86-specific list that refer to the same
> device node, both pending for addition later. On VT switch back, the first one
> (the already removed one) will be added successfully, the second one (the
> still plugged-in one) fails. Since the fd is correct, the device works until
> it is removed again. The removed devices' config_info (i.e. the syspath)
> doesn't match the actual device we addded tough (the input number increases
> with each plug), it doesn't get removed, the fd remains open and we lose track
> of the fd count. Plugging the device in again leads to a dead device.
>
> Fix this by adding a call to notify the DDX to purge any remainders of devices
> with the given config_info, that's the only identifiable bit we have at this
> point.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=97928
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Xi/stubs.c | 14 ++++++++++++++
> config/config.c | 2 ++
> hw/dmx/dmxinput.c | 5 +++++
> hw/kdrive/src/kinput.c | 5 +++++
> hw/xfree86/common/xf86Xinput.c | 19 +++++++++++++++++++
> hw/xquartz/darwinXinput.c | 15 +++++++++++++++
> include/input.h | 1 +
> 7 files changed, 61 insertions(+)
>
> diff --git a/Xi/stubs.c b/Xi/stubs.c
> index 39bee7c..27848a2 100644
> --- a/Xi/stubs.c
> +++ b/Xi/stubs.c
> @@ -143,3 +143,17 @@ DeleteInputDeviceRequest(DeviceIntPtr dev)
> {
> RemoveDevice(dev, TRUE);
> }
> +
> +/****************************************************************************
> + *
> + * Caller: configRemoveDevice (and others)
> + *
> + * Remove any traces of the input device specified in config_info.
> + * This is only necessary if the ddx keeps information around beyond
> + * the NewInputDeviceRequest/DeleteInputDeviceRequest
> + *
> + */
> +void
> +RemoveInputDeviceTraces(const char *config_info)
> +{
> +}
> diff --git a/config/config.c b/config/config.c
> index 1fb368c..fb60295 100644
> --- a/config/config.c
> +++ b/config/config.c
> @@ -107,6 +107,8 @@ remove_devices(const char *backend, const char *config_info)
> if (dev->config_info && strcmp(dev->config_info, config_info) == 0)
> remove_device(backend, dev);
> }
> +
> + RemoveInputDeviceTraces(config_info);
> }
>
> BOOL
> diff --git a/hw/dmx/dmxinput.c b/hw/dmx/dmxinput.c
> index 4ccb439..d201034 100644
> --- a/hw/dmx/dmxinput.c
> +++ b/hw/dmx/dmxinput.c
> @@ -123,3 +123,8 @@ void
> DeleteInputDeviceRequest(DeviceIntPtr pDev)
> {
> }
> +
> +void
> +RemoveInputDeviceTraces(const char *config_info)
> +{
> +}
> diff --git a/hw/kdrive/src/kinput.c b/hw/kdrive/src/kinput.c
> index 2c39624..8b08747 100644
> --- a/hw/kdrive/src/kinput.c
> +++ b/hw/kdrive/src/kinput.c
> @@ -2302,3 +2302,8 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
> {
> RemoveDevice(pDev, TRUE);
> }
> +
> +void
> +RemoveInputDeviceTraces(const char *config_info)
> +{
> +}
> diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> index 0095272..d673711 100644
> --- a/hw/xfree86/common/xf86Xinput.c
> +++ b/hw/xfree86/common/xf86Xinput.c
> @@ -1119,6 +1119,25 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
> input_unlock();
> }
>
> +void
> +RemoveInputDeviceTraces(const char *config_info)
> +{
> + PausedInputDevicePtr d, tmp;
> +
> + if (new_input_devices_list.next == NULL &&
> + new_input_devices_list.prev == NULL)
> + xorg_list_init(&new_input_devices_list);
This bit is unnecessary, as you already initialize
new_input_devices_list when you declare it at the top of the file.
With these 3 lines dropped the entire series looks good to me and is:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
> +
> + xorg_list_for_each_entry_safe(d, tmp, &new_input_devices_list, node) {
> + const char *ci = xf86findOptionValue(d->pInfo->options, "config_info");
> + if (!ci || strcmp(ci, config_info) != 0)
> + continue;
> +
> + xorg_list_del(&d->node);
> + free(d);
> + }
> +}
> +
> /*
> * convenient functions to post events
> */
> diff --git a/hw/xquartz/darwinXinput.c b/hw/xquartz/darwinXinput.c
> index 3efaa2c..fea7e92 100644
> --- a/hw/xquartz/darwinXinput.c
> +++ b/hw/xquartz/darwinXinput.c
> @@ -147,3 +147,18 @@ DeleteInputDeviceRequest(DeviceIntPtr dev)
> {
> DEBUG_LOG("DeleteInputDeviceRequest(%p)\n", dev);
> }
> +
> +/****************************************************************************
> + *
> + * Caller: configRemoveDevice (and others)
> + *
> + * Remove any traces of the input device specified in config_info.
> + * This is only necessary if the ddx keeps information around beyond
> + * the NewInputDeviceRequest/DeleteInputDeviceRequest
> + *
> + */
> +void
> +RemoveInputDeviceTraces(const char *config_info)
> +{
> + DEBUG_LOG("RemoveInputDeviceTraces(%s)\n", config_info);
> +}
> diff --git a/include/input.h b/include/input.h
> index c7b1e91..bb58b22 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -635,6 +635,7 @@ extern _X_EXPORT int NewInputDeviceRequest(InputOption *options,
> InputAttributes * attrs,
> DeviceIntPtr *dev);
> extern _X_EXPORT void DeleteInputDeviceRequest(DeviceIntPtr dev);
> +extern _X_EXPORT void RemoveInputDeviceTraces(const char *config_info);
>
> extern _X_EXPORT void DDXRingBell(int volume, int pitch, int duration);
>
>
More information about the xorg-devel
mailing list