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

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 {

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;
 

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

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