Fwd: Re: [PATCH 1/1] include/basetsd.h: fix bad casting

Max TenEyck Woodbury max at mtew.isa-geek.net
Sat Oct 13 06:10:02 CDT 2012


On 10/12/2012 10:25 PM, Dan Kegel wrote:
> Hi Max,
> here's a little test program that shows that your patch changes how
> UIntToPtr works:
> 
> #include <stdio.h>
> #include <stdint.h>
> 
> #define UIntToPtrA(i)             ((void *)(intptr_t)((unsigned int)i))
> #define UIntToPtrB(i)             ((void *)(intptr_t)(i))
> 
> int main(int argc, char **argv)
> {
>     short x = 65534;
>     printf("%d -> %p\n", x, UIntToPtrA(x));
>     printf("%d -> %p\n", x, UIntToPtrB(x));
>     return 0;
> }
> 
> On a 64 bit machine, this outputs
> 
> -2 -> 0xfffffffe
> -2 -> 0xfffffffffffffffe
> 
> This could change the behavior of functions that call UIntToPtr, like
> say EnumResourceTypesA().
> 
> I would suggest writing a conformance test to expose whether the
> change brings Wine closer to native behavior, or further away, adding
> that to your patch, and then verifying with winetestbot that the patch
> improves wine's conformance.
> 
> My guess is that the current behavior is right, but hey, the test will
> show what's really up.
> - Dan
> 

I 'greped' the entire wine tree for uses of these macros.  There was
only a couple places where the argument was an expression and in those
cases I did not see that the change would cause problems.

Frankly, I do not think I could come up with a test where the change
would make a difference.  I am submitting the patch against the weird
possibility that such a case does exist.

The change is strictly preventative maintenance with just a hint of
aesthetic improvement to make them coordinate with the 'Handle' variants
of these macros.

Now, if you are going to tell me that these definitions were *copied*
from a Microsoft *source* rather than derived from a Microsoft
specification, you would have a point, but then there would be a whole
bunch of copyright issues that would need to be worked through.





More information about the wine-devel mailing list