[PATCH 04/10] When activating an explicit grab, update owning listener

[Chase Douglas]

On 04/19/2012 09:49 AM, Bryce Harrington wrote:
> On Tue, Apr 17, 2012 at 04:33:23PM -0700, Chase Douglas wrote:
>> Pointer passive grabs may be changed by the grabbing client. This allows
>> for a selecting client to change an implicit grab to an active grab,
>> which is the mechanism used for pop-up windows like application menus.
>>
>> We need to do the same thing with touches. If the grabbing client is the
>> owner of a touch sequence, change the listener record to reflect the new
>> grab. If the grabbing client is not the owner, nothing changes for the
>> touch.
>>
>> Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
>> ---
>>  dix/events.c |   33 +++++++++++++++++++++++++++++++++
>>  1 files changed, 33 insertions(+), 0 deletions(-)
>>
>> diff --git a/dix/events.c b/dix/events.c
>> index 52ce0b8..9496b6f 100644
>> --- a/dix/events.c
>> +++ b/dix/events.c
>> @@ -1409,6 +1409,38 @@ ReattachToOldMaster(DeviceIntPtr dev)
>>  }
>>  
>>  /**
>> + * Update touch records when an explicit grab is activated. Any touches owned by
>> + * the grabbing client are updated so the listener state reflects the new grab.
>> + */
>> +static void
>> +UpdateTouchesForGrab(DeviceIntPtr mouse)
>> +{
>> +    int i;
>> +
>> +    if (!mouse->touch || mouse->deviceGrab.fromPassiveGrab)
>> +        return;
> 
> This routine also has an assumption that !mouse.  Which I suppose is
> pretty obvious, but the function will definitely segfault if mouse==NULL
> here.

I think it's a safe assumption based on what the purpose of the function is.

>> +    for (i = 0; i < mouse->touch->num_touches; i++) {
>> +        TouchPointInfoPtr ti = mouse->touch->touches + i;
>> +        GrabPtr grab = mouse->deviceGrab.grab;
>> +
>> +        if (ti->active &&
>> +            CLIENT_BITS(ti->listeners[0].listener) == grab->resource) {
>> +            ti->listeners[0].listener = grab->resource;
> 
> Checking len(ti->listeners) prior to this spot would avoid out of
> bounds, if that's a conceivable situation.

The check for ti->active should ensure that ti->listeners[0] is valid.
There must be at least one listener for the dix touch record to still be
valid. Perhaps we should remove ti->active and replace all checks with
ti->num_listeners > 0. That would remove the duplicity of ti->active and
ensure we don't get into a broken state.

Thanks!

-- Chase