Re: [PATCH] Input - mt: Fix input_mt_get_slot_by_key

Henrik Rydberg
2015-02-03T19:30:27Z

Hi Benjamin,

> Slot 0 is released correclty, but when we look for Contact ID 2, the slot
> 0 is then picked up again because it is marked as inactive (trackingID < 0).
>
> This is wrong, and we should not reuse a slot in the same frame.
> The test should also check for input_mt_is_used().

Good catch! However...

> @@ -453,7 +456,7 @@ int input_mt_get_slot_by_key(struct input_dev *dev, int key)
> return s - mt->slots;
>
> for (s = mt->slots; s != mt->slots + mt->num_slots; s++)
> - if (!input_mt_is_active(s)) {
> + if (!input_mt_is_active(s) && !input_mt_is_used(mt, s)) {
> s->key = key;
> return s - mt->slots;
> }
>

Here, you are changing the preconditions of the function without explicit
reference to all its users. For one, it is now assumed that input_mt_is_used()
is up-to-date, which requires either input_mt_drop_unused() or
input_mt_sync_frame(), which does not seem to be true for all users of
input_mt_get_slot_by_key(). After a couple of iterations with
input_mt_report_slot_state() in those drivers, input_mt_is_used() will be true
for all slots, and the driver will stop working.

How about defering the deassignments until the end of the loop instead? That
would remove possible reuse.

Thanks,
Henrik