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

[Peter Hutterer]

On Fri, Oct 21, 2016 at 11:08:04AM +0200, Hans de Goede wrote:
> 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(+)

[...]

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

ah, right. leftover from a previous version, sorry. thanks for the review.

Cheers,
   Peter
> 
> 
> > +
> > +    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);
> > 
> >