[PATCH libXi] 1UL may be 4 bytes, force to 1ULL

Jeremy Huddleston jeremyhu at apple.com
Sun Oct 2 01:19:12 PDT 2011


Peter, any thoughts on this?

On Sep 20, 2011, at 10:01, Jeremy Huddleston wrote:

> I just realized I made (at least one) error in the inline functions I wrote below.  s/< 0/< 1/ ... there're probably others, but that just emphasizes my point that there should be conversion routines rather than repeating this code with possible error in other locations.
> 
> On Sep 19, 2011, at 7:03 PM, Jeremy Huddleston wrote:
> 
>> I'm curious... why is the cls_wire->increment.frac getting cast to unsigned int?  It's already typed as uint32_t.
>> 
>> Looking at the context a bit more, I agree with Jamey that this should work (with or without explicit casts):
>> 
>> cls_lib->increment  = (double)cls_wire->increment.integral;
>> cls_lib->increment += ldexp((double)cls_wire->increment.frac, -32);
>> 
>> But I think it would be more useful to provide something like this in the proto to help with conversion:
>> 
>> static inline double fp1616_to_double(FP1616 in) {
>>   double ret;
>>   ret = (double)(in >> 16);
>>   ret += ldexp((double)(in & 0xff), -16);
>>   return ret;
>> }
>> 
>> static inline double fp3232_to_double(FP3232 in) {
>>   double ret;
>>   ret = (double)in.integral;
>>   ret += ldexp((double)in.frac, -32);
>>   return ret;
>> }
>> 
>> 
>> static FP1616 double double_to_fp1616(double in) {
>>   FP1616 ret;
>>   int32_t integral
>>   double frac_f;
>>   uint32_t frac_d;
>> 
>>   integral = (int32_t)in << 16;
>>   frac_f = in - (double)((int32_t)(in));
>> 
>>   if (fabs(frac_f) < 0) {
>>     frac_d = (uint32_t)ldexp(frac_f,16);
>>   } else {
>>     /* crap */
>>     frac_d = 0;
>>   }
>> 
>>   ret = integral << 16;
>>   ret |= frac & 0xff;
>>   return ret;
>> }
>> 
>> static FP3232 double double_to_fp3232(double in) {
>>   FP3232 ret;
>>   int32_t integral
>>   double frac_f;
>>   uint32_t frac_d;
>> 
>>   integral = (int32_t)in << 16;
>>   frac_f = in - (double)((int32_t)(in));
>> 
>>   if (fabs(frac_f) < 0) {
>>     frac_d = (uint32_t)ldexp(frac_f,16);
>>   } else {
>>     /* crap */
>>     frac_d = 0;
>>   }
>> 
>>   ret.integral = integral;
>>   ret.frac = frac_d;
>>   return ret;
>> }
>> 
>> On Sep 19, 2011, at 5:37 PM, Jamey Sharp wrote:
>> 
>>> I'm a bit skeptical... Would ldexp(frac, -32) work for you? Otherwise
>>> I'd argue for (UINT64_C(1) << 32).
>>> 
>>> Jamey
>>> 
>>> On Tue, Sep 20, 2011 at 10:19:54AM +1000, Peter Hutterer wrote:
>>>> Reported-by: Jeremy Huddleston <jeremyhu at apple.com>
>>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>>> ---
>>>> something like this good enough?
>>>> 
>>>> src/XExtInt.c |    2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/src/XExtInt.c b/src/XExtInt.c
>>>> index d74a8d4..4009285 100644
>>>> --- a/src/XExtInt.c
>>>> +++ b/src/XExtInt.c
>>>> @@ -1577,7 +1577,7 @@ copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int *nclasses)
>>>>                   cls_lib->scroll_type= cls_wire->scroll_type;
>>>>                   cls_lib->flags      = cls_wire->flags;
>>>>                   cls_lib->increment  = cls_wire->increment.integral;
>>>> -                    cls_lib->increment += (unsigned int)cls_wire->increment.frac/(double)(1UL << 32);
>>>> +                    cls_lib->increment += (unsigned int)cls_wire->increment.frac/(double)(1ULL << 32);
>>>> 
>>>>                   to->classes[cls_idx++] = any_lib;
>>>>               }
>>>> -- 
>>>> 1.7.6
>>>> 
>>>> _______________________________________________
>>>> xorg-devel at lists.x.org: X.Org development
>>>> Archives: http://lists.x.org/archives/xorg-devel
>>>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>> 
> 
> ---
> Jeremy Huddleston
> 
> Rebuild Sudan
> - Board of Directors
> - http://www.rebuildsudan.org
> 
> Berkeley Foundation for Opportunities in Information Technology
> - Advisory Board
> - http://www.bfoit.org
> 



More information about the xorg-devel mailing list