Signed-off-by: Serge Gautherie winehq-git_serge_180711@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(); }
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
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.
On Fri, Feb 28, 2020 at 4:11 AM Serge Gautherie winehq-git_serge_180711@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
On 01/03/2020 23:34, Alex Henrie wrote:
On Fri, Feb 28, 2020 at 4:11 AM Serge Gautherie winehq-git_serge_180711@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.
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");elseok(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...
On Sun, Mar 1, 2020 at 3:42 PM Zebediah Figura z.figura12@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
On 3/1/20 6:58 PM, Alex Henrie wrote:
On Sun, Mar 1, 2020 at 3:42 PM Zebediah Figura z.figura12@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
-
Alex Henrie -
Marvin -
Serge Gautherie -
Zebediah Figura