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

[James Tabor]

Just curious, why does that just look so wrong to me.......

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=e3459ec36ecd783d67c27d...

commit e3459ec36ecd783d67c27dac4e81ad9facc08605
Author: Mark Jansen <mark.jansen at reactos.org
http://www.reactos.org/mailman/listinfo/ros-diffs>
AuthorDate: Sat Jan 6 21:09:51 2018 +0100

[PSDK] Fix definition of INVALID_PROCESSTRACE_HANDLE as suggested by Thomas.
    #267
---
 sdk/include/psdk/evntrace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sdk/include/psdk/evntrace.h b/sdk/include/psdk/evntrace.h
index dbbc660963..7056f119fa 100644
--- a/sdk/include/psdk/evntrace.h
+++ b/sdk/include/psdk/evntrace.h
@@ -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)

Did this break three of the build bots?

[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).a...
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