Re: [ros-dev] [ros-diffs] [tkreuzer] 63247: [DDK/XDK/NTOSKRNL/DRIVERS] Fix incompatible definition of a number of NTOSKRNL data imports. These imports are declared in MS DDK in a way that is usually not how you would declar...

[Thomas Faber]

Woohoo, thanks!

Questions/comments inline.

On 2014-05-12 01:11, [e-mail] wrote:
> --- trunk/reactos/include/ddk/ntifs.h	[iso-8859-1] (original)
> +++ trunk/reactos/include/ddk/ntifs.h	[iso-8859-1] Sun May 11 23:11:16 2014
> @@ -870,11 +870,11 @@
>  (NTAPI *PRTL_FREE_STRING_ROUTINE)(
>    _In_ __drv_freesMem(Mem) _Post_invalid_ PVOID Buffer);
>
> -extern const PRTL_ALLOCATE_STRING_ROUTINE RtlAllocateStringRoutine;
> -extern const PRTL_FREE_STRING_ROUTINE RtlFreeStringRoutine;
> +extern NTKERNELAPI const PRTL_ALLOCATE_STRING_ROUTINE RtlAllocateStringRoutine;
> +extern NTKERNELAPI const PRTL_FREE_STRING_ROUTINE RtlFreeStringRoutine;
>
> #if _WIN32_WINNT >= 0x0600
> -extern const PRTL_REALLOCATE_STRING_ROUTINE RtlReallocateStringRoutine;
> +extern NTKERNELAPI const PRTL_REALLOCATE_STRING_ROUTINE RtlReallocateStringRoutine;
>  #endif
>
> _Function_class_(RTL_HEAP_COMMIT_ROUTINE)

Do you have any idea what RtlAllocateStringRoutine and friends are?
They don't seem to exist. (NDK defines them for user mode btw, without
dllimport)
> Modified: trunk/reactos/include/ddk/wdm.h
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ddk/wdm.h?rev=63247...
> ==============================================================================
> --- trunk/reactos/include/ddk/wdm.h	[iso-8859-1] (original)
> +++ trunk/reactos/include/ddk/wdm.h	[iso-8859-1] Sun May 11 23:11:16 2014
> @@ -82,10 +85,13 @@
>  #endif
>
> /* For ReactOS */
> -#if !defined(_NTOSKRNL_) && !defined(_BLDR_)
> +#if !defined(_NTOSKRNL_) && !defined(_BLDR_) && !defined(_NTSYSTEM_)
>  #define NTKERNELAPI DECLSPEC_IMPORT
>  #else
>  #define NTKERNELAPI
> +#ifndef _NTSYSTEM_
> +#define _NTSYSTEM_
> +#endif
>  #endif
>
> #if defined(_X86_) && !defined(_NTHAL_)

We should probably switch to using _NTSYSTEM_ instead of _NTOSKRNL_ at
some point?
> @@ -129,6 +135,29 @@
>  #define ALLOC_DATA_PRAGMA 1
>  #endif
>
> +#endif /* _MSC_VER */
>
> +/* These macros are used to create aliases for imported data. We need to do
> this to have declarations that are compatible with MS DDK */
>
> +#ifdef _M_IX86
> +#define __SYMBOL(_Name) "_"#_Name
> +#define __IMPORTSYMBOL(_Name) "__imp__"#_Name
> +#define __IMPORTNAME(_Name) _imp__##_Name
> +#else
> +#define __SYMBOL(_Name) #_Name
> +#define __IMPORTSYMBOL(_Name) "__imp_"#_Name
> +#define __IMPORTNAME(_Name) __imp_##_Name
> +#endif
> +#ifdef _MSC_VER
> +#define __CREATE_NTOS_DATA_IMPORT_ALIAS(_Name) \
> __pragma(comment(linker, "/alternatename:"__SYMBOL(_Name) "=" __IMPORTSYMBOL(_Name)))
>
> +#else /* !_MSC_VER */
> +#ifndef __STRINGIFY
> +#define __STRINGIFY(_exp) #_exp
> +#endif
> +#define _Pragma_redifine_extname(_Name, _Target) _Pragma(__STRINGIFY(redefine_extname _Name _Target))
> +#define __CREATE_NTOS_DATA_IMPORT_ALIAS(_Name) \
> _Pragma_redifine_extname(_Name,__IMPORTNAME(_Name))
>
> #endif
>
> #if defined(_WIN64)

s/redifine/redefine/
> --- trunk/reactos/ntoskrnl/kd64/kdapi.c	[iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/kd64/kdapi.c	[iso-8859-1] Sun May 11

23:11:16 2014
> @@ -1903,9 +1903,7 @@
>          /* We are disabled now */
>          KdDebuggerEnabled = FALSE;
>
> -#undef KdDebuggerEnabled
>              SharedUserData->KdDebuggerEnabled = FALSE;
> -#define KdDebuggerEnabled _KdDebuggerEnabled
>          }
>       }

Should this not use KD_DEBUGGER_ENABLED?
(same in kdinit.c)

[Timo Kreuzer]

Am 12.05.2014 08:12, schrieb Thomas Faber:
> > +extern NTKERNELAPI const PRTL_FREE_STRING_ROUTINE RtlFreeStringRoutine;
> >
> > #if _WIN32_WINNT >= 0x0600
> > -extern const PRTL_REALLOCATE_STRING_ROUTINE RtlReallocateStringRoutine;
> > +extern NTKERNELAPI const PRTL_REALLOCATE_STRING_ROUTINE RtlReallocateStringRoutine;
> >  #endif
> >
> > _Function_class_(RTL_HEAP_COMMIT_ROUTINE)
>
> Do you have any idea what RtlAllocateStringRoutine and friends are?
> They don't seem to exist. (NDK defines them for user mode btw, without
> dllimport)

Hmm, looks like not even Vista or 7 export them. Maybe it's located in
some library and supposed to be per module. I have no idea, maybe Alex
knows. I'll remove the NTKERNELAPI
> > Modified: trunk/reactos/include/ddk/wdm.h
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ddk/wdm.h?rev=63247...
> > ==============================================================================
> > --- trunk/reactos/include/ddk/wdm.h	[iso-8859-1] (original)
> > +++ trunk/reactos/include/ddk/wdm.h	[iso-8859-1] Sun May 11 23:11:16 2014
> > @@ -82,10 +85,13 @@
> >  #endif
> >
> > /* For ReactOS */
> > -#if !defined(_NTOSKRNL_) && !defined(_BLDR_)
> > +#if !defined(_NTOSKRNL_) && !defined(_BLDR_) && !defined(_NTSYSTEM_)
> >  #define NTKERNELAPI DECLSPEC_IMPORT
> >  #else
> >  #define NTKERNELAPI
> > +#ifndef _NTSYSTEM_
> > +#define _NTSYSTEM_
> > +#endif
> >  #endif
> >
> > #if defined(_X86_) && !defined(_NTHAL_)
>
> We should probably switch to using _NTSYSTEM_ instead of _NTOSKRNL_ at
> some point?

MS actually uses _NTOSP_ for this. They might define _NTSYSTEM_ for
multiple modules. I just used what is used the same way in WDK headers,
but there it's only in one place (NlsMbCodePageTag). The other locations
are probably "filtered" away. But since we use the header for building
ntoskrnl itself, we need it for the other definitions as well.
> > --- trunk/reactos/ntoskrnl/kd64/kdapi.c	[iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/kd64/kdapi.c	[iso-8859-1] Sun May 11
>
> 23:11:16 2014
> > @@ -1903,9 +1903,7 @@
> >          /* We are disabled now */
> >          KdDebuggerEnabled = FALSE;
> >
> > -#undef KdDebuggerEnabled
> >              SharedUserData->KdDebuggerEnabled = FALSE;
> > -#define KdDebuggerEnabled _KdDebuggerEnabled
> >          }
> >       }
>
> Should this not use KD_DEBUGGER_ENABLED?
> (same in kdinit.c)

We could do it, it's just not neccessary. The definition is probably
there to allow compilation of the same code for use inside and outside
of ntoskrnl. Since this is always in ntoskrnl, it's not required.