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

[Dmitry Timoshkov]

Max TenEyck Woodbury  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.

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

[Max TenEyck Woodbury]

On 10/12/2012 01:46 PM, Dmitry Timoshkov wrote:
> Max TenEyck Woodbury  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.

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

[Dmitry Timoshkov]

Max TenEyck Woodbury  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.

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

[David Laight]

On Sat, Oct 13, 2012 at 10:49:49AM +0900, Dmitry Timoshkov wrote:
> Max TenEyck Woodbury  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.

This example is a bit bogus but shows what he means:
with:
    struct foo { int a[2] } bar;
expand:
    IntToPtr(bar + 4)

I can't actually remember what all the perverted type names above
actually are! I had a feeling that INT_PTR had something do do with
an integer that is big enough to hold the value of a pointer rather
than being a pointer to an INT, but that means that having both
INT_PTR and LONG_PTR is silly.

Having an extra typedef for 'foo *' is really silly, amongst other 
things you can't convert it to 'const foo *' (only to 'foo * const').

David