[PATCH] fix a sign problem with valuator data.

Chase Douglas chase.douglas at canonical.com
Fri Oct 15 08:45:07 PDT 2010


On Thu, 2010-10-14 at 15:09 -0400, Joe Shaw wrote:
> From: Joe Shaw <joeshaw at litl.com>
> 
> Without this patch, any negative valuator value is wrong when returned
> from XQueryDeviceState().  This is a regression from at least xserver
> 1.4.
> 
> Valuator data is set in dix/getevents.c:set_valuators() by copying
> signed int values into an unsigned int field
> DeviceEvent.valuators.data.
> 
> That data is converted into a double with an implicit cast by
> assignment to axisVal[i] in Xi/exevents.c:UpdateDeviceState().
> 
> That double is converted back to a signed int in
> queryst.c:ProcXQueryDeviceState().  If the original value in
> set_valuators() is negative, the double value will be > 2^31 and the
> conversion back to a signed int is undefined.  (Although I
> consistently see the value -2^31.)
> 
> Fix this by changing the definition of DeviceEvent.valuators.data from
> uint32_t to int32_t.
> 
> Signed-off-by: Joe Shaw <joeshaw at litl.com>
> ---
>  dix/getevents.c    |    2 +-
>  include/eventstr.h |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/dix/getevents.c b/dix/getevents.c
> index 82bb77b..531595e 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -177,7 +177,7 @@ set_valuators(DeviceIntPtr dev, DeviceEvent* event, int first_valuator,
>      }
>  
>      memcpy(&event->valuators.data[first_valuator],
> -           valuators, num_valuators * sizeof(uint32_t));
> +           valuators, num_valuators * sizeof(int32_t));
>  
>  }
>  
> diff --git a/include/eventstr.h b/include/eventstr.h
> index 433227e..377cceb 100644
> --- a/include/eventstr.h
> +++ b/include/eventstr.h
> @@ -99,7 +99,7 @@ struct _DeviceEvent
>      struct {
>          uint8_t  mask[(MAX_VALUATORS + 7)/8]; /**< Valuator mask */
>          uint8_t  mode[(MAX_VALUATORS + 7)/8]; /**< Valuator mode (Abs or Rel)*/
> -        uint32_t data[MAX_VALUATORS];         /**< Valuator data */
> +        int32_t  data[MAX_VALUATORS];         /**< Valuator data */
>          int32_t  data_frac[MAX_VALUATORS];    /**< Fractional part for data */
>      } valuators;
>      struct {

I can't think of any reason this fix would cause issues except for
values > 2^31. I don't think that ever happens :).

Reviewed-by: Chase Douglas <chase.douglas at canonical.com>



More information about the xorg-devel mailing list