[PATCH xserver] inputthread: Re-add fd to the inputThreadInfo->fds pollfd set when re-added
Peter Hutterer
peter.hutterer at who-t.net
Fri Oct 14 04:38:00 UTC 2016
On Wed, Oct 12, 2016 at 04:55:08PM +0200, Hans de Goede wrote:
> If the following call sequence happens:
> 1) InputThreadUnregisterDev(fd)
> 2) close(fd)
> 3) fd = open(...) /* returns same fd as just closed */
> 4) InputThreadRegisterDev(fd, ...)
>
> Without InputThreadDoWork(); running in the mean time, then we would
> keep the closed fd in the inputThreadInfo->fds pollfd set, rather then
> removing it and adding the new one, causing some devices to not work
> after a vt-switch when using xf86-input-evdev.
>
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=97880
> Reported-and-tested-by: Mihail Konev <k.mvc at ya.ru>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> os/inputthread.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/os/inputthread.c b/os/inputthread.c
> index ab1559f..c20c21c 100644
> --- a/os/inputthread.c
> +++ b/os/inputthread.c
> @@ -49,6 +49,7 @@ Bool InputThreadEnable = TRUE;
>
> typedef enum _InputDeviceState {
> device_state_added,
> + device_state_re_added,
> device_state_running,
> device_state_removed
> } InputDeviceState;
> @@ -206,8 +207,8 @@ InputThreadRegisterDev(int fd,
> if (dev) {
> dev->readInputProc = readInputProc;
> dev->readInputArgs = readInputArgs;
> - /* Override possible unhandled state == device_state_removed */
> - dev->state = device_state_running;
> + if (dev->state == device_state_removed)
> + dev->state = device_state_re_added;
I'm wondering, especially with the other patch in mind:
https://patchwork.freedesktop.org/patch/113763/
how about we just treat InputThreadDevice in state device_state_removed as
"invisible" to InputThreadRegisterDev, i.e. we don't re-use the struct but
allocate a new one. This way we have the old fd automatically removed and
the new one added as expected, skipping the need for the
device_state_re_added handling.
This would be only a one-line change a bit above this hunk
- if (old->fd == fd) {
+ if (old->fd == fd && dev->state != device_state_removed) {
The only drawback is that we rely on xorg_list_append() and that the new
entry is later than the previous one so we have the same remove/add order as
in your device_state_re_added handling below. That needs a comment
but other than that we should get the same result?
Cheers,
Peter
> } else {
> dev = calloc(1, sizeof(InputThreadDevice));
> if (dev == NULL) {
> @@ -344,6 +345,16 @@ InputThreadDoWork(void *arg)
> ospoll_listen(inputThreadInfo->fds, dev->fd, X_NOTIFY_READ);
> dev->state = device_state_running;
> break;
> + case device_state_re_added:
> + /* Device might use a new fd with the same number */
> + ospoll_remove(inputThreadInfo->fds, dev->fd);
> + ospoll_add(inputThreadInfo->fds, dev->fd,
> + ospoll_trigger_level,
> + InputReady,
> + dev);
> + ospoll_listen(inputThreadInfo->fds, dev->fd, X_NOTIFY_READ);
> + dev->state = device_state_running;
> + break;
> case device_state_running:
> break;
> case device_state_removed:
> --
> 2.9.3
More information about the xorg-devel
mailing list