[PATCH] Don't alter device button maps in DoSetPointerMapping

[Peter Hutterer]

On Sun, Jan 04, 2009 at 11:31:47AM -0500, Thomas Jaeger wrote:
> I think the current behavior violates the core spec: If, say, button 1
> is pressed on a device where it is currently disabled, but it is mapped
> to itself on the VCP, then an XSetPointerMapping request that doesn't
> change the mapping of button 1 will nonetheless fail.

Oh right. AFAICT, this is a leftover for when we copied the SD into the MD
each time the SD changes. In this case, the SD's button map would be merged
into the MD, thus overwriting the last XSetPointerMapping request (unless said
request is also applied to the SD).
        
We don't do that anymore now, and the MD in fact uses a standalone mapping, so
the approach is good. However, see below:

> From e021ca2981f6e01ff23523106ab42f73bcbe7308 Mon Sep 17 00:00:00 2001
> From: Thomas Jaeger <ThJaeger at gmail.com>
> Date: Sun, 4 Jan 2009 11:22:40 -0500
> Subject: [PATCH] Don't alter device button maps in DoSetPointerMapping
> 
> Currently, if a device map differs from the core pointer map, then the
> request may return MappingBusy, even though all the affected core
> buttons are in the up state.
> ---
>  dix/devices.c |   34 ++++++++++------------------------
>  1 files changed, 10 insertions(+), 24 deletions(-)
> 
> diff --git a/dix/devices.c b/dix/devices.c
> index df6cd51..44a4f18 100644
> --- a/dix/devices.c
> +++ b/dix/devices.c
> @@ -1829,36 +1829,23 @@ static int
>  DoSetPointerMapping(ClientPtr client, DeviceIntPtr device, BYTE *map, int n)
>  {
>      int rc, i = 0;
> -    DeviceIntPtr dev = NULL;
>  
>      if (!device || !device->button)
>          return BadDevice;
>  
> -    for (dev = inputInfo.devices; dev; dev = dev->next) {
> -        if ((dev->coreEvents || dev == inputInfo.pointer) && dev->button) {
> -	    rc = XaceHook(XACE_DEVICE_ACCESS, client, dev, DixManageAccess);
> -	    if (rc != Success)
> -		return rc;
> -	}
> -    }
> +    rc = XaceHook(XACE_DEVICE_ACCESS, client, device, DixManageAccess);
> +    if (rc != Success)
> +        return rc;
>  
> -    for (dev = inputInfo.devices; dev; dev = dev->next) {
> -        if ((dev->coreEvents || dev == inputInfo.pointer) && dev->button) {
> -            for (i = 0; i < n; i++) {
> -                if ((device->button->map[i + 1] != map[i]) &&
> -                    BitIsOn(device->button->down, i + 1)) {
> -                    return MappingBusy;
> -                }
> -            }
> +    for (i = 0; i < n; i++) {
> +        if ((device->button->map[i + 1] != map[i]) &&
> +                BitIsOn(device->button->down, i + 1)) {
> +            return MappingBusy;
>          }
>      }
>  
> -    for (dev = inputInfo.devices; dev; dev = dev->next) {
> -        if ((dev->coreEvents || dev == inputInfo.pointer) && dev->button) {
> -            for (i = 0; i < n; i++)
> -                dev->button->map[i + 1] = map[i];
> -        }
> -    }
> +    for (i = 0; i < n; i++)
> +        device->button->map[i + 1] = map[i];
>  
>      return Success;
>  }
> @@ -1869,7 +1856,7 @@ ProcSetPointerMapping(ClientPtr client)
>      BYTE *map;
>      int ret;
>      int i, j;
> -    DeviceIntPtr ptr = PickPointer(client);
> +    DeviceIntPtr ptr = inputInfo.pointer;

No. PickPointer must be used here, as it return's the client's ClientPointer.
Hardcoding the VCP means that a client may change the button mapping of the
wrong pointer. So while this approach would work for 1.6 and master in the
single-pointer case, it breaks once a second MD is available.

Cheers,
  Peter