From f8fba5b5884bfb62225b9bde991dfdbec92ad751 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 1 Apr 2014 15:32:46 +1000 Subject: [PATCH] Drop hardcoded MAX_SLOTS in favour of pre-allocated memory We can't allocate in sync_mt_state since it may be called in the signal handler. So pre-allocate based on the device's number of slots, store that in the libevdev struct and use it for the sync process. This fixes a remaining bug with the handling of ABS_MT_TRACKING_ID. If a device had > MAX_SLOTS and a slot above that limit would start or stop during a SYN_DROPPED event, the slot would not be synced, and a subsequent touch in that slot may double-terminate or double-open a touchpoint in the client. For the effects of that see commit 41334b5b40cd5456f5f584b55d8888aaafa1f26e Author: Peter Hutterer Date: Thu Mar 6 11:54:00 2014 +1000 If the tracking ID changes during SYN_DROPPED, terminate the touch first Signed-off-by: Peter Hutterer Reviewed-by: Benjamin Tissoires --- libevdev/libevdev-int.h | 15 ++++- libevdev/libevdev.c | 50 ++++++++++++----- libevdev/libevdev.h | 5 -- test/test-libevdev-events.c | 107 ------------------------------------ 4 files changed, 49 insertions(+), 128 deletions(-) diff --git a/libevdev/libevdev-int.h b/libevdev/libevdev-int.h index 5ff6026..98c75ce 100644 --- a/libevdev/libevdev-int.h +++ b/libevdev/libevdev-int.h @@ -32,7 +32,6 @@ #include "libevdev-util.h" #define MAX_NAME 256 -#define MAX_SLOTS 60 #define ABS_MT_MIN ABS_MT_SLOT #define ABS_MT_MAX ABS_MT_TOOL_Y #define ABS_MT_CNT (ABS_MT_MAX - ABS_MT_MIN + 1) @@ -57,6 +56,11 @@ enum SyncState { SYNC_IN_PROGRESS, }; +struct mt_sync_state { + int code; + int val[]; +}; + struct libevdev { int fd; bool initialized; @@ -94,6 +98,15 @@ struct libevdev { size_t queue_nsync; /**< number of sync events */ struct timeval last_event_time; + + struct { + struct mt_sync_state *mt_state; + size_t mt_state_sz; /* in bytes */ + unsigned long *slot_update; + size_t slot_update_sz; /* in bytes */ + unsigned long *tracking_id_changes; + size_t tracking_id_changes_sz; /* in bytes */ + } mt_sync; }; struct logdata { diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c index e2070d4..572fd37 100644 --- a/libevdev/libevdev.c +++ b/libevdev/libevdev.c @@ -152,6 +152,9 @@ libevdev_reset(struct libevdev *dev) free(dev->phys); free(dev->uniq); free(dev->mt_slot_vals); + free(dev->mt_sync.mt_state); + free(dev->mt_sync.tracking_id_changes); + free(dev->mt_sync.slot_update); memset(dev, 0, sizeof(*dev)); dev->fd = -1; dev->initialized = false; @@ -389,8 +392,24 @@ libevdev_set_fd(struct libevdev* dev, int fd) goto out; } dev->current_slot = abs_info.value; - } + dev->mt_sync.mt_state_sz = sizeof(*dev->mt_sync.mt_state) + + (dev->num_slots) * sizeof(int); + dev->mt_sync.mt_state = calloc(1, dev->mt_sync.mt_state_sz); + + dev->mt_sync.tracking_id_changes_sz = NLONGS(dev->num_slots) * sizeof(long); + dev->mt_sync.tracking_id_changes = malloc(dev->mt_sync.tracking_id_changes_sz); + + dev->mt_sync.slot_update_sz = NLONGS(dev->num_slots * ABS_MT_CNT) * sizeof(long); + dev->mt_sync.slot_update = malloc(dev->mt_sync.slot_update_sz); + + if (!dev->mt_sync.tracking_id_changes || + !dev->mt_sync.slot_update || + !dev->mt_sync.mt_state) { + rc = -ENOMEM; + goto out; + } + } } } @@ -556,14 +575,15 @@ sync_mt_state(struct libevdev *dev, int create_events) int axis, slot; int ioctl_success = 0; int last_reported_slot = 0; - struct mt_state { - int code; - int val[MAX_SLOTS]; - } mt_state; - unsigned long slot_update[NLONGS(MAX_SLOTS * ABS_MT_CNT)] = {0}; - unsigned long tracking_id_changes[NLONGS(MAX_SLOTS)] = {0}; + struct mt_sync_state *mt_state = dev->mt_sync.mt_state; + unsigned long *slot_update = dev->mt_sync.slot_update; + unsigned long *tracking_id_changes = dev->mt_sync.tracking_id_changes; int need_tracking_id_changes = 0; + memset(dev->mt_sync.slot_update, 0, dev->mt_sync.slot_update_sz); + memset(dev->mt_sync.tracking_id_changes, 0, + dev->mt_sync.tracking_id_changes_sz); + #define AXISBIT(_slot, _axis) (_slot * ABS_MT_CNT + _axis - ABS_MT_MIN) for (axis = ABS_MT_MIN; axis <= ABS_MT_MAX; axis++) { @@ -573,8 +593,8 @@ sync_mt_state(struct libevdev *dev, int create_events) if (!libevdev_has_event_code(dev, EV_ABS, axis)) continue; - mt_state.code = axis; - rc = ioctl(dev->fd, EVIOCGMTSLOTS(sizeof(struct mt_state)), &mt_state); + mt_state->code = axis; + rc = ioctl(dev->fd, EVIOCGMTSLOTS(dev->mt_sync.mt_state_sz), mt_state); if (rc < 0) { /* if the first ioctl fails with -EINVAL, chances are the kernel doesn't support the ioctl. Simply continue */ @@ -586,19 +606,19 @@ sync_mt_state(struct libevdev *dev, int create_events) if (ioctl_success == 0) ioctl_success = 1; - for (slot = 0; slot < min(dev->num_slots, MAX_SLOTS); slot++) { + for (slot = 0; slot < dev->num_slots; slot++) { - if (*slot_value(dev, slot, axis) == mt_state.val[slot]) + if (*slot_value(dev, slot, axis) == mt_state->val[slot]) continue; if (axis == ABS_MT_TRACKING_ID && *slot_value(dev, slot, axis) != -1 && - mt_state.val[slot] != -1) { + mt_state->val[slot] != -1) { set_bit(tracking_id_changes, slot); need_tracking_id_changes = 1; } - *slot_value(dev, slot, axis) = mt_state.val[slot]; + *slot_value(dev, slot, axis) = mt_state->val[slot]; set_bit(slot_update, AXISBIT(slot, axis)); /* note that this slot has updates */ @@ -615,7 +635,7 @@ sync_mt_state(struct libevdev *dev, int create_events) } if (need_tracking_id_changes) { - for (slot = 0; slot < min(dev->num_slots, MAX_SLOTS); slot++) { + for (slot = 0; slot < dev->num_slots; slot++) { if (!bit_is_set(tracking_id_changes, slot)) continue; @@ -631,7 +651,7 @@ sync_mt_state(struct libevdev *dev, int create_events) init_event(dev, ev, EV_SYN, SYN_REPORT, 0); } - for (slot = 0; slot < min(dev->num_slots, MAX_SLOTS); slot++) { + for (slot = 0; slot < dev->num_slots; slot++) { if (!bit_is_set(slot_update, AXISBIT(slot, ABS_MT_SLOT))) continue; diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h index 77ee08a..aa7b90d 100644 --- a/libevdev/libevdev.h +++ b/libevdev/libevdev.h @@ -914,11 +914,6 @@ enum libevdev_read_status { * have been synced. For more details on what libevdev does to sync after a * SYN_DROPPED event, see @ref syn_dropped. * - * @note The implementation of libevdev limits the maximum number of slots - * that can be synched. If your device exceeds the number of slots - * (currently 60), slot indices equal and above this maximum are ignored and - * their value will not update until the next event in that slot. - * * If a device needs to be synced by the caller but the caller does not call * with the @ref LIBEVDEV_READ_FLAG_SYNC flag set, all events from the diff are * dropped after libevdev updates its internal state and event processing diff --git a/test/test-libevdev-events.c b/test/test-libevdev-events.c index 412db71..fda7617 100644 --- a/test/test-libevdev-events.c +++ b/test/test-libevdev-events.c @@ -30,8 +30,6 @@ #include "test-common.h" -#define MAX_SLOTS 60 /* as in libevdev-int.h */ - START_TEST(test_next_event) { struct uinput_device* uidev; @@ -662,110 +660,6 @@ START_TEST(test_syn_delta_mt_reset_slot) } END_TEST -START_TEST(test_syn_delta_mt_too_many) -{ - struct uinput_device* uidev; - struct libevdev *dev; - int rc; - struct input_event ev; - struct input_absinfo abs[6]; - int i; - int num_slots = MAX_SLOTS + 20; - - memset(abs, 0, sizeof(abs)); - abs[0].value = ABS_X; - abs[0].maximum = 1000; - abs[1].value = ABS_MT_POSITION_X; - abs[1].maximum = 1000; - - abs[2].value = ABS_Y; - abs[2].maximum = 1000; - abs[3].value = ABS_MT_POSITION_Y; - abs[3].maximum = 1000; - - abs[4].value = ABS_MT_SLOT; - abs[4].maximum = num_slots; - abs[5].value = ABS_MT_TOOL_Y; - abs[5].maximum = 500; - - rc = test_create_abs_device(&uidev, &dev, - 6, abs, - EV_SYN, SYN_REPORT, - -1); - ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc)); - - for (i = num_slots; i >= 0; i--) { - uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, i); - uinput_device_event(uidev, EV_ABS, ABS_X, 100 + i); - uinput_device_event(uidev, EV_ABS, ABS_Y, 500 + i); - uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 100 + i); - uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 500 + i); - uinput_device_event(uidev, EV_ABS, ABS_MT_TOOL_Y, 1 + i); - uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); - } - - /* drain the fd, so libevdev_next_event doesn't pick up any events - before the FORCE_SYNC */ - do { - rc = read(libevdev_get_fd(dev), &ev, sizeof(ev)); - } while (rc > 0); - - rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_FORCE_SYNC, &ev); - ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC); - - i = 0; - while (i < num_slots) { - int slot; - - rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev); - ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC); - - if (libevdev_event_is_code(&ev, EV_SYN, SYN_REPORT)) - break; - - if (libevdev_event_is_code(&ev, EV_ABS, ABS_X) || - libevdev_event_is_code(&ev, EV_ABS, ABS_Y)) - continue; - - ck_assert_int_eq(ev.type, EV_ABS); - ck_assert_int_eq(ev.code, ABS_MT_SLOT); - slot = ev.value; - ck_assert_int_lt(slot, MAX_SLOTS); - - rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev); - - /* last MT_SLOT event is on its own */ - if (libevdev_event_is_code(&ev, EV_SYN, SYN_REPORT)) - break; - - ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC); - ck_assert_int_eq(ev.type, EV_ABS); - ck_assert_int_eq(ev.code, ABS_MT_POSITION_X); - ck_assert_int_eq(ev.value, 100 + slot); - rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev); - ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC); - ck_assert_int_eq(ev.type, EV_ABS); - ck_assert_int_eq(ev.code, ABS_MT_POSITION_Y); - ck_assert_int_eq(ev.value, 500 + slot); - rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev); - ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC); - ck_assert_int_eq(ev.type, EV_ABS); - ck_assert_int_eq(ev.code, ABS_MT_TOOL_Y); - ck_assert_int_eq(ev.value, 1 + slot); - - i++; - } - - /* we expect eactly MAX_SLOTS to be synced */ - ck_assert_int_eq(i, MAX_SLOTS); - rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev); - ck_assert_int_eq(rc, -EAGAIN); - - uinput_device_free(uidev); - libevdev_free(dev); -} -END_TEST - START_TEST(test_syn_delta_led) { struct uinput_device* uidev; @@ -1806,7 +1700,6 @@ libevdev_events(void) tcase_add_test(tc, test_syn_delta_button); tcase_add_test(tc, test_syn_delta_abs); tcase_add_test(tc, test_syn_delta_mt); - tcase_add_test(tc, test_syn_delta_mt_too_many); tcase_add_test(tc, test_syn_delta_mt_reset_slot); tcase_add_test(tc, test_syn_delta_led); tcase_add_test(tc, test_syn_delta_sw);