[PATCH] fix a sign problem with valuator data.
Peter Hutterer
peter.hutterer at who-t.net
Sun Oct 17 17:17:33 PDT 2010
On Fri, Oct 15, 2010 at 11:45:07AM -0400, Chase Douglas wrote:
> 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>
oops. looks like a typo, should have probably been the other way round
(data_frac as unsigned and data as signed).
merged, thanks.
Cheers,
Peter
More information about the xorg-devel
mailing list