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 <peter.hutterer@who-t.net>
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 <peter.hutterer@who-t.net>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>

diff --git a/libevdev/libevdev-int.h b/libevdev/libevdev-int.h
index 5ff6026..98c75ce 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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);