ui/input-linux.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
The evdev devices in input-linux.c are read in blocks of one whole
event. If there are not enough bytes available, they are discarded,
instead of being kept for the next read operation. This results in
lost events, of even non-working devices.
This patch keeps track of the number of bytes to be read to fill up
a whole event, and then handle it.
Signed-off-by: Javier Celaya <jcelaya@gmail.com>
---
ui/input-linux.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/ui/input-linux.c b/ui/input-linux.c
index ac31f47719..02b0d4b2fe 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -169,6 +169,8 @@ struct InputLinux {
bool has_abs_x;
int num_keys;
int num_btns;
+ struct input_event event;
+ int to_be_read;
QTAILQ_ENTRY(InputLinux) next;
};
@@ -327,25 +329,30 @@ static void input_linux_handle_mouse(InputLinux *il, struct input_event *event)
static void input_linux_event(void *opaque)
{
InputLinux *il = opaque;
- struct input_event event;
int rc;
+ int offset;
+ uint8_t *p = (uint8_t *)&il->event;
for (;;) {
- rc = read(il->fd, &event, sizeof(event));
- if (rc != sizeof(event)) {
+ offset = sizeof(il->event) - il->to_be_read;
+ rc = read(il->fd, &p[offset], il->to_be_read);
+ if (rc != il->to_be_read) {
if (rc < 0 && errno != EAGAIN) {
fprintf(stderr, "%s: read: %s\n", __func__, strerror(errno));
qemu_set_fd_handler(il->fd, NULL, NULL, NULL);
close(il->fd);
+ } else if (rc > 0){
+ il->to_be_read -= rc;
}
break;
}
+ il->to_be_read = sizeof(il->event);
if (il->num_keys) {
- input_linux_handle_keyboard(il, &event);
+ input_linux_handle_keyboard(il, &il->event);
}
if (il->has_rel_x && il->num_btns) {
- input_linux_handle_mouse(il, &event);
+ input_linux_handle_mouse(il, &il->event);
}
}
}
@@ -417,6 +424,7 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
}
}
+ il->to_be_read = sizeof(il->event);
qemu_set_fd_handler(il->fd, input_linux_event, NULL, il);
if (il->keycount) {
/* delay grab until all keys are released */
--
2.11.0
On So, 2017-03-26 at 11:53 +0200, Javier Celaya wrote: > The evdev devices in input-linux.c are read in blocks of one whole > event. If there are not enough bytes available, they are discarded, > instead of being kept for the next read operation. This results in > lost events, of even non-working devices. Have you seen this happening in practice? > + struct input_event event; > + int to_be_read; I'd suggest to store offset (i.e. bytes already read) instead, should make the whole logic a bit simpler and easier to read. > + } else if (rc > 0){ checkpatch.pl complains here: ERROR: space required before the open brace '{' cheers, Gerd
Hi Javi 2017-03-27 12:11 GMT+02:00 Gerd Hoffmann <kraxel@redhat.com>: > On So, 2017-03-26 at 11:53 +0200, Javier Celaya wrote: > > The evdev devices in input-linux.c are read in blocks of one whole > > event. If there are not enough bytes available, they are discarded, > > instead of being kept for the next read operation. This results in > > lost events, of even non-working devices. > > Have you seen this happening in practice? > Yes, quite frequently, like once per hour. Totally destroys a good gaming session :) The curious thing is, the mouse stops working, but in the keyboard I see some missing keyup events (the keys get stuck), but then it recovers. > > > + struct input_event event; > > + int to_be_read; > > I'd suggest to store offset (i.e. bytes already read) instead, should > make the whole logic a bit simpler and easier to read. > OK > > > + } else if (rc > 0){ > > checkpatch.pl complains here: > ERROR: space required before the open brace '{' > Oops, missed that > > cheers, > Gerd > >
Hi Javier, can you add a line to explain what did you change between v1/v2? Thank, Phil. On 03/26/2017 06:53 AM, Javier Celaya wrote: > The evdev devices in input-linux.c are read in blocks of one whole > event. If there are not enough bytes available, they are discarded, > instead of being kept for the next read operation. This results in > lost events, of even non-working devices. > > This patch keeps track of the number of bytes to be read to fill up > a whole event, and then handle it. > > Signed-off-by: Javier Celaya <jcelaya@gmail.com> > --- > ui/input-linux.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/ui/input-linux.c b/ui/input-linux.c > index ac31f47719..02b0d4b2fe 100644 > --- a/ui/input-linux.c > +++ b/ui/input-linux.c > @@ -169,6 +169,8 @@ struct InputLinux { > bool has_abs_x; > int num_keys; > int num_btns; > + struct input_event event; > + int to_be_read; > > QTAILQ_ENTRY(InputLinux) next; > }; > @@ -327,25 +329,30 @@ static void input_linux_handle_mouse(InputLinux *il, struct input_event *event) > static void input_linux_event(void *opaque) > { > InputLinux *il = opaque; > - struct input_event event; > int rc; > + int offset; > + uint8_t *p = (uint8_t *)&il->event; > > for (;;) { > - rc = read(il->fd, &event, sizeof(event)); > - if (rc != sizeof(event)) { > + offset = sizeof(il->event) - il->to_be_read; > + rc = read(il->fd, &p[offset], il->to_be_read); > + if (rc != il->to_be_read) { > if (rc < 0 && errno != EAGAIN) { > fprintf(stderr, "%s: read: %s\n", __func__, strerror(errno)); > qemu_set_fd_handler(il->fd, NULL, NULL, NULL); > close(il->fd); > + } else if (rc > 0){ > + il->to_be_read -= rc; > } > break; > } > + il->to_be_read = sizeof(il->event); > > if (il->num_keys) { > - input_linux_handle_keyboard(il, &event); > + input_linux_handle_keyboard(il, &il->event); > } > if (il->has_rel_x && il->num_btns) { > - input_linux_handle_mouse(il, &event); > + input_linux_handle_mouse(il, &il->event); > } > } > } > @@ -417,6 +424,7 @@ static void input_linux_complete(UserCreatable *uc, Error **errp) > } > } > > + il->to_be_read = sizeof(il->event); > qemu_set_fd_handler(il->fd, input_linux_event, NULL, il); > if (il->keycount) { > /* delay grab until all keys are released */ >
© 2016 - 2024 Red Hat, Inc.