Re: [ros-dev] [ros-diffs] [reactos] 01/02: [PSDK] Fix definition of INVALID_PROCESSTRACE_HANDLE as suggested by Thomas. #267

[Thomas Faber]

On 2018-01-08 08:17, James Tabor wrote:
>   @@ -711,7 +711,7 @@ typedef struct
> _ENABLE_TRACE_PARAMETERS {
>      PEVENT_FILTER_DESCRIPTOR EnableFilterDesc;
>    } ENABLE_TRACE_PARAMETERS, *PENABLE_TRACE_PARAMETERS;
>  
>  -#define INVALID_PROCESSTRACE_HANDLE ((TRACEHANDLE)INVALID_HANDLE_VALUE)
>  +#define INVALID_PROCESSTRACE_HANDLE ((TRACEHANDLE)(ULONG_PTR)INVALID_HANDLE_VALUE)
>  
>    #if defined(UNICODE) || defined(_UNICODE)
>  
>  
>   From W7U SDK rc:
>  
>  sdkinc.v7rc/Evntrace.h:93:typedef ULONG64 TRACEHANDLE, *PTRACEHANDLE;
>  sdkinc.v7rc/Evntrace.h:1500:#define INVALID_PROCESSTRACE_HANDLE
> ((TRACEHANDLE)INVALID_HANDLE_VALUE) 
Yes, this is quite unusual on Microsoft's side, they normally don't make
this mistake in their headers. Casting directly from HANDLE (aka void *)
to TRACEHANDLE (aka unsigned __int64) makes the result dependent on
whether the compiler treats pointers as "signed" or "unsigned". This
is
not at all standardized, which is why GCC issues a warning
about this (cast from pointer to integer of different size). Modern
Microsoft compilers treat it as unsigned, which is why I suggested the
step to ULONG_PTR in between to force this behavior across compilers.

My guess is that older MS compilers treated it as signed, which is why
MS had to introduce the funny note about the return value here:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa364089(v=vs.85).…
It might also be why they never changed their headers: in order to avoid
defining either of the values as "correct".

>   Did this break three of the build bots? 
It's the commit after that broke build. In local builds (which use PCH
by default), advapi32.h is automatically included. Buildbot doesn't use
PCH, so it includes wmistr.h before anything else, which that header
doesn't seem to like.

Best,
Thomas