[PATCH 1/2] ntdll/tests: Create a test for RtlGetNtProductType().

[Serge Gautherie]

Signed-off-by: Serge Gautherie <winehq-git_serge_180711(a)gautherie.fr>
---
 dlls/ntdll/tests/rtl.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c
index 445f353..e39e1df 100644
--- a/dlls/ntdll/tests/rtl.c
+++ b/dlls/ntdll/tests/rtl.c
@@ -27,6 +27,12 @@
 #include "in6addr.h"
 #include "inaddr.h"
 
+/* See test_RtlGetNtProductType(), where it does not actually help */
+/* #define TEST_WITH_SEH */
+#ifdef TEST_WITH_SEH
+#include "wine/exception.h"
+#endif
+
 #ifndef __WINE_WINTERNL_H
 
 typedef struct _RTL_HANDLE
@@ -112,6 +118,7 @@ static NTSTATUS  (WINAPI *pRtlMakeSelfRelativeSD)(PSECURITY_DESCRIPTOR,PSECURITY
 static NTSTATUS  (WINAPI *pRtlAbsoluteToSelfRelativeSD)(PSECURITY_DESCRIPTOR,PSECURITY_DESCRIPTOR,PULONG);
 static NTSTATUS  (WINAPI *pLdrRegisterDllNotification)(ULONG, PLDR_DLL_NOTIFICATION_FUNCTION, void *, void **);
 static NTSTATUS  (WINAPI *pLdrUnregisterDllNotification)(void *);
+static BOOLEAN   (WINAPI *pRtlGetNtProductType)(LPDWORD);
 
 static HMODULE hkernel32 = 0;
 static BOOL      (WINAPI *pIsWow64Process)(HANDLE, PBOOL);
@@ -183,6 +190,7 @@ static void InitFunctionPtrs(void)
         pRtlAbsoluteToSelfRelativeSD = (void *)GetProcAddress(hntdll, "RtlAbsoluteToSelfRelativeSD");
         pLdrRegisterDllNotification = (void *)GetProcAddress(hntdll, "LdrRegisterDllNotification");
         pLdrUnregisterDllNotification = (void *)GetProcAddress(hntdll, "LdrUnregisterDllNotification");
+        pRtlGetNtProductType = (void *)GetProcAddress(hntdll, "RtlGetNtProductType");
     }
     hkernel32 = LoadLibraryA("kernel32.dll");
     ok(hkernel32 != 0, "LoadLibrary failed\n");
@@ -3673,6 +3681,46 @@ static void test_LdrRegisterDllNotification(void)
     pLdrUnregisterDllNotification(cookie);
 }
 
+static void test_RtlGetNtProductType(void)
+{
+    DWORD type;
+
+    if (!pRtlGetNtProductType)
+    {
+        win_skip("RtlGetNtProductType is not available\n");
+        return;
+    }
+
+    /* NULL is not a special value for this function */
+#ifdef TEST_WITH_SEH
+    /* With SEH, address is different, but it still crashes on Windows:
+     * 'rtl: unhandled exception c0000005 at 00000001'
+     */
+    __TRY
+    {
+        BOOLEAN ret;
+
+        ret = pRtlGetNtProductType(NULL);
+
+        if (ret)
+            ok(FALSE, "RtlGetNtProductType(NULL) succeeded\n");
+        else
+            ok(FALSE, "RtlGetNtProductType(NULL) did not crash\n");
+    }
+    __EXCEPT(EXCEPTION_EXECUTE_HANDLER)
+    {
+        /* As expected */
+        ok(TRUE, "\n");
+    }
+    __ENDTRY
+#endif
+
+    type = 0xdeadbeef;
+    ok(pRtlGetNtProductType(&type), "RtlGetNtProductType failed\n");
+    ok(type >= VER_NT_WORKSTATION && type <= VER_NT_SERVER, "unknown type %u\n", type);
+    trace("NtProductType: %u\n", type);
+}
+
 START_TEST(rtl)
 {
     InitFunctionPtrs();
@@ -3713,4 +3761,5 @@ START_TEST(rtl)
     test_LdrEnumerateLoadedModules();
     test_RtlMakeSelfRelativeSD();
     test_LdrRegisterDllNotification();
+    test_RtlGetNtProductType();
 }
-- 
2.10.0.windows.1

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=65974

Your paranoid android.


=== w1064v1809 (32 bit report) ===

ntdll:
0670:rtl: unhandled exception c0000005 at 77D89D4B

=== w1064v1809_2scr (32 bit report) ===

ntdll:
1a04:rtl: unhandled exception c0000005 at 77BA9D4B

=== w1064v1809_ar (32 bit report) ===

ntdll:
1854:rtl: unhandled exception c0000005 at 77219D4B

=== w1064v1809_he (32 bit report) ===

ntdll:
194c:rtl: unhandled exception c0000005 at 77D39D4B

=== w1064v1809_ja (32 bit report) ===

ntdll:
1968:rtl: unhandled exception c0000005 at 772B9D4B

=== w1064v1809_zh_CN (32 bit report) ===

ntdll:
18bc:rtl: unhandled exception c0000005 at 77979D4B

[Serge Gautherie]

See https://testbot.winehq.org/JobDetails.pl?Key=65973 :
w1064v1709 and w1064v1809 do crash on 32 bit (only).

Yet, I believe my test is correct, as far as I know.

--

[Alex Henrie]

On Fri, Feb 28, 2020 at 4:11 AM Serge Gautherie
<winehq-git_serge_180711(a)gautherie.fr> wrote:
> +/* See test_RtlGetNtProductType(), where it does not actually help */
> +/* #define TEST_WITH_SEH */

In my opinion the patch would be better without these two comments.
> +static BOOLEAN   (WINAPI *pRtlGetNtProductType)(LPDWORD);

Please write DWORD * instead of LPDWORD.

Other than that, the patch looks good to me, thanks.

-Alex

[Serge Gautherie]

On 01/03/2020 23:34, Alex Henrie wrote:
> On Fri, Feb 28, 2020 at 4:11 AM Serge Gautherie
> <winehq-git_serge_180711(a)gautherie.fr>  wrote:
> > +/* See test_RtlGetNtProductType(), where it does not actually help */
> > +/* #define TEST_WITH_SEH */
>
> In my opinion the patch would be better without these two comments.

Agreed, I'll rename the define to be more explicit instead.
> > +static BOOLEAN   (WINAPI *pRtlGetNtProductType)(LPDWORD);
>
> Please write DWORD * instead of LPDWORD.

Why? LPDWORD is a copy of the declaration. Moreover, it is used on other 
function pointers like pRtlMakeSelfRelativeSD.

--

[Zebediah Figura]

Hello Serge,

On 2/28/20 5:10 AM, Serge Gautherie wrote:
> @@ -3673,6 +3681,46 @@ static void test_LdrRegisterDllNotification(void)
>      pLdrUnregisterDllNotification(cookie);
>  }
>
> +static void test_RtlGetNtProductType(void)
> +{
> +    DWORD type;
> +
> +    if (!pRtlGetNtProductType)
> +    {
> +        win_skip("RtlGetNtProductType is not available\n");
> +        return;
> +    }
> +
> +    /* NULL is not a special value for this function */

This comment seems unnecessary, given the code.
> +#ifdef TEST_WITH_SEH
> +    /* With SEH, address is different, but it still crashes on Windows:
> +     * 'rtl: unhandled exception c0000005 at 00000001'
> +     */

This seems like an internal detail, and so not interesting enough to
comment.
> +    __TRY
> +    {
> +        BOOLEAN ret;
> +
> +        ret = pRtlGetNtProductType(NULL);
> +
> +        if (ret)
> +            ok(FALSE, "RtlGetNtProductType(NULL) succeeded\n");
> +        else
> +            ok(FALSE, "RtlGetNtProductType(NULL) did not crash\n");

This branch seems pointless.
> +    }
> +    __EXCEPT(EXCEPTION_EXECUTE_HANDLER)
> +    {
> +        /* As expected */

This comment seems unnecessary.
> +        ok(TRUE, "\n");

And ok(TRUE) does nothing, so there's no point in having it.
> +    }
> +    __ENDTRY
> +#endif

Does a program actually depend on this function crashing? If not, I
suspect it would be better to leave off this test entirely, or add
something like the following, as is done elsewhere in Wine tests:

if (0)
{
    /* crashes on Windows */
    ret = pRtlGetNtProductType(NULL);
}
> +
> +    type = 0xdeadbeef;
> +    ok(pRtlGetNtProductType(&type), "RtlGetNtProductType failed\n");
> +    ok(type >= VER_NT_WORKSTATION && type <= VER_NT_SERVER, "unknown type %u\n", type);
> +    trace("NtProductType: %u\n", type);
> +}
> +

Is this trace() really worth having? We already have too many...

[Alex Henrie]

On Sun, Mar 1, 2020 at 3:42 PM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> And ok(TRUE) does nothing, so there's no point in having it.

On the contrary, ok(TRUE) increments the number of passing tests ;-)

-Alex

[Zebediah Figura]

On 3/1/20 6:58 PM, Alex Henrie wrote:
> On Sun, Mar 1, 2020 at 3:42 PM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> > And ok(TRUE) does nothing, so there's no point in having it.
>
> On the contrary, ok(TRUE) increments the number of passing tests ;-)

Good point ;-)
> -Alex