Re: [PATCH] Input - mt: Fix input_mt_get_slot_by_key

[Henrik Rydberg]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/