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

[Max TenEyck Woodbury]

On 10/12/2012 09:49 PM, Dmitry Timoshkov wrote:
> Max TenEyck Woodbury <max at mtew.isa-geek.net> wrote:
> 
>>>> -#define IntToPtr(i)             ((void *)(INT_PTR)((INT)i))
>>>> -#define UIntToPtr(ui)           ((void *)(UINT_PTR)((UINT)ui))
>>>> -#define LongToPtr(l)            ((void *)(LONG_PTR)((LONG)l))
>>>> -#define ULongToPtr(ul)          ((void *)(ULONG_PTR)((ULONG)ul))
>>>> +#define IntToPtr(i)             ((void *)(INT_PTR)(i))
>>>> +#define UIntToPtr(ui)           ((void *)(UINT_PTR)(ui))
>>>> +#define LongToPtr(l)            ((void *)(LONG_PTR)(l))
>>>> +#define ULongToPtr(ul)          ((void *)(ULONG_PTR)(ul))
>>>
>>> You forgot to explain what's bad with it.
>>>
>>
>> The original applied the extra conversion to ONLY THE FIRST
>> component of the expression, not to the WHOLE expression.  That
>> can (although it's not likely to) cause hard to diagnose problems.
> 
> What 'the first component' are you talking about? This code is win32 only.
> 
'i', 'ui', 'l' and 'ul' are parameters to the macros,  When the macros
are used, you put in an expression as the argument to the macro.  As
originally written for example, the cast to LONG in ((LONG)l) is
applied to only the first component of the expression substituted in
place of 'l'.  In order to get the cast to apply to the whole
expression, you need parenthesis around the parameter.  That is
literally (l).  Then the cast applies to the whole expression.  That is
(LONG_PTR)(l) casts the entire expression substituted into 'l' to a
LONG_PTR value.  The macros that convert to 'Handle' rather than 'Ptr'
do this properly.

In the vast majority of cases the expression substituted into 'l' is
a simple variable and there is no problem.  In almost all of the
remaining cases, casting the first element of the expression coerces
the rest of the expression to the proper type.  In rare cases, other
elements of the expression might force the expression's value to some
other type.  In most of those cases the cast to LONG_PTR fixes the
problem.  In a really odd case there might be a problem caused by the
extra cast to LONG.  Since that cast is buried in a macro definition,
it will be difficult to spot that cast as the source of such a problem
from the source code.

The original definitions include three casts in sequence.  The first of
those casts is done improperly.  Even if it were done properly, the next
cast in the sequence makes it redundant.  I removed the improper cast
since it IS redundant.

NUFF SAID?