[PATCH] Touch: Fix duplicate TouchBegin selection with virtual devices

[Chase Douglas]

On Thu, Sep 27, 2012 at 11:34 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Fri, Sep 07, 2012 at 06:30:23PM +0100, Daniel Stone wrote:
>> Given the following scenario:
>>   1) client A selects for TouchBegin on window W for device D
>>   2) client B selects for TouchBegin on window W for XIAllDevices
>>   3) client C selects for TouchBegin on window W with device E
>>
>> Step 3 will fail with BadImplementation, because attempting to look up
>> XIAllDevices or XIAllMasterDevices with dixLookupDevices doesn't work.
>> This should succeed (or, if it was selecting for device D, fail with
>> BadAccess as it would be a duplicate selection).
>>
>> Fix this by performing the appropriate lookup for virtual devices.
>>
>> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
>> Cc: Peter Hutterer <peter.hutterer at who-t.net>
>> Cc: Chase Douglas <chase.douglas at ubuntu.com>
>> ---
>>  Xi/xiselectev.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/Xi/xiselectev.c b/Xi/xiselectev.c
>> index 0e45cb8..ab1b624 100644
>> --- a/Xi/xiselectev.c
>> +++ b/Xi/xiselectev.c
>> @@ -180,8 +180,13 @@ ProcXISelectEvents(ClientPtr client)
>>                      if (CLIENT_ID(iclient->resource) == client->index)
>>                          continue;
>>
>> -                    dixLookupDevice(&tmp, evmask->deviceid, serverClient,
>> -                                    DixReadAccess);
>> +                    if (evmask->deviceid == XIAllDevices)
>> +                        tmp = inputInfo.all_devices;
>> +                    else if (evmask->deviceid == XIAllMasterDevices)
>> +                        tmp = inputInfo.all_master_devices;
>> +                    else
>> +                        dixLookupDevice(&tmp, evmask->deviceid, serverClient,
>> +                                        DixReadAccess);
>>                      if (!tmp)
>>                          return BadImplementation;       /* this shouldn't happen */
>>
>> --
>> 1.7.10.4
>
> We may have to document the protocol better.
>
> The protocol states that if there is overlapping selection, we must return
> BadAccess. With the patch above added, we get BadAccess if the first client
> registers for XIAll(Master)Devices and the second client registers for
> anything. But the inverse, the first client registering for a device and the
> second client registering for XIAll(Master)Devices succeeds.
>
> Now, I think this behaviour makes sense but it needs to be documented
> explicitely. Plus, we'd have to test that the server actually behaves that
> way, because I'm pretty sure right now it doesn't.

This sounds dangerous. It could lead to subtle race conditions where
behavior depends on which client request reaches the server first.

Why does this behavior seem better to you than a strict mutual exclusion policy?

-- Chase