[PATCH v2] Xi: fix modifier offset in XIPassiveGrab swapping function

[Peter Hutterer]

On 25/01/2014 08:48 , Alan Coopersmith wrote:
> On 01/24/14 12:05 AM, Peter Hutterer wrote:
>> The request is followed by mask_len 4-byte units, then followed by the
>> actual
>> modifiers.
>>
>> Also fix up the swapping test, which had the same issue.
>>
>> Reported-by: Alan Coopersmith <alan.coopersmith at oracle.com>
>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>> ---
>> Changes to v1:
>> - turns out the swapping test has the same bug, which explains why it
>> didn't
>>    trigger
>>
>>   Xi/xipassivegrab.c                      | 2 +-
>>   test/xi2/protocol-xipassivegrabdevice.c | 9 ++++++++-
>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
>> index eccec0a..8aba977 100644
>> --- a/Xi/xipassivegrab.c
>> +++ b/Xi/xipassivegrab.c
>> @@ -63,7 +63,7 @@ SProcXIPassiveGrabDevice(ClientPtr client)
>>       swaps(&stuff->mask_len);
>>       swaps(&stuff->num_modifiers);
>>
>> -    mods = (uint32_t *) &stuff[1];
>> +    mods = (uint32_t *) &stuff[1] + stuff->mask_len;
>>
>>       for (i = 0; i < stuff->num_modifiers; i++, mods++) {
>>           swapl(mods);
>> diff --git a/test/xi2/protocol-xipassivegrabdevice.c
>> b/test/xi2/protocol-xipassivegrabdevice.c
>> index 1e2341e..31ef3d2 100644
>> --- a/test/xi2/protocol-xipassivegrabdevice.c
>> +++ b/test/xi2/protocol-xipassivegrabdevice.c
>> @@ -137,6 +137,7 @@ request_XIPassiveGrabDevice(ClientPtr client,
>> xXIPassiveGrabDeviceReq * req,
>>   {
>>       int rc;
>>       int local_modifiers;
>> +    int mask_len;
>>
>>       rc = ProcXIPassiveGrabDevice(&client_request);
>>       assert(rc == error);
>> @@ -153,10 +154,11 @@ request_XIPassiveGrabDevice(ClientPtr client,
>> xXIPassiveGrabDeviceReq * req,
>>       swaps(&req->deviceid);
>>       local_modifiers = req->num_modifiers;
>>       swaps(&req->num_modifiers);
>> +    mask_len = req->mask_len;
>>       swaps(&req->mask_len);
>>
>>       while (local_modifiers--) {
>> -        CARD32 *mod = ((CARD32 *) (req + 1)) + local_modifiers;
>> +        CARD32 *mod = ((CARD32 *) (req + 1) + mask_len) +
>> local_modifiers;
>
> Doesn't that need to be outside the parens that cast to (CARD32) so it
> adds in CARD32-sized units instead of in xXIPassiveGrabDeviceReq sized
> units?
>
> i.e. ((CARD32 *) (req + 1)) + mask_len + local_modifiers;

Oh, right, that would make it more obvious. but the effect is the same, 
isn't it? as long as it's not inside the (req + 1) condition, the first 
arg is already cast and the + 1 is a 4-byte.

so (int*)a + 1 + 1 is the same as ((int*)a + 1) + 1, at least that's 
what my quick test here showed.

I'll fix it up locally though.

Cheers,
   Peter