[PATCH xserver 3/3] ddx: add new call to purge input devices that weren't added

[Hans de Goede]

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);
>
>