some improvement for createprocess tests: - no longer assuming using Win7 console handles - fixed regression introduced by shell-no-window type of consoles - added some more tests around CreateProcess
V3 -> V4: removed skip part
Marvin will report errors for the 3 tests, until this bug is fixed https://bugs.winehq.org/show_bug.cgi?id=52847 I'll let the NLS Guru(s) take care of it
This testbot run (all patches in this serie + ugly UTF8 support in TranslateCharsetInfo to partially implement) show that fixing above bug is the only remaining generator of failures https://testbot.winehq.org/JobDetails.pl?Key=113102S
Signed-off-by: Eric Pouech eric.pouech@gmail.com ---
Eric Pouech (3): dlls/kernel32/tests: let the console tests pass if current console is shell-no-window dlls/kernel32/tests: adapt dup console handle test to non Win7 handles dlls/kernel32/tests: test when passing non inheritable handles in CreateProcess
dlls/kernel32/tests/process.c | 115 +++++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 15 deletions(-)
This patch shall let this kernel32:process test pass when run from TestBot.
Note: adapted the tests to non Win7 type of handles (the ones with 2 lower bits set...). Windows 8 and above use real handles for console. Wine has been doing so since since 2 years.
The is_console() helper is not testing what its name pretends. It actually tests whether we're running the test on W7 :-( So let's get rid of it.
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/kernel32/tests/process.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 8f18a0d37a7..9629b50a744 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -1541,8 +1541,13 @@ static void test_Console(void) startup.hStdOutput = CreateFileA("CONOUT$", GENERIC_READ|GENERIC_WRITE, 0, &sa, OPEN_EXISTING, 0, 0);
/* first, we need to be sure we're attached to a console */ - if (!is_console(startup.hStdInput) || !is_console(startup.hStdOutput)) + if (startup.hStdInput == INVALID_HANDLE_VALUE || startup.hStdOutput == INVALID_HANDLE_VALUE) { + /* this fails either when this test process is run detached from console + * (unlikely, as this very process must be explicitely created with detached flag), + * or is attached to a Wine's shell-no-window kind of console (if the later, detach from it) + */ + FreeConsole(); /* we're not attached to a console, let's do it */ AllocConsole(); startup.hStdInput = CreateFileA("CONIN$", GENERIC_READ|GENERIC_WRITE, 0, &sa, OPEN_EXISTING, 0, 0);
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=113103
Your paranoid android.
=== debian11 (32 bit Hindi:India report) ===
kernel32: process.c:1587: Test failed: GetLastError: expecting 87 got 6 process.c:1597: Test failed: GetLastError: expecting 87 got 6
This test hadn't been changed with console rewrite. The status before this patch: - it's not run on Windows except Win7 - it's not run on Wine
Rewrite it so that it run on all cases and inverse the test logic (it was only testing Win7 behavior, which is no longer what we want).
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/kernel32/tests/process.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 9629b50a744..ebadc2e8d12 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -1507,11 +1507,6 @@ static void test_DebuggingFlag(void) DeleteFileA(resfile); }
-static BOOL is_console(HANDLE h) -{ - return h != INVALID_HANDLE_VALUE && ((ULONG_PTR)h & 3) == 3; -} - static void test_Console(void) { char buffer[2 * MAX_PATH + 35]; @@ -2500,17 +2495,11 @@ static void test_DuplicateHandle(void) DeleteFileA(file_name);
f = CreateFileA("CONIN$", GENERIC_READ|GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); - if (!is_console(f)) - { - skip("DuplicateHandle on console handle\n"); - CloseHandle(f); - return; - } - + ok(f != INVALID_HANDLE_VALUE, "Failed to open CONIN$ %lu\n", GetLastError()); r = DuplicateHandle(GetCurrentProcess(), f, GetCurrentProcess(), &out, 0, FALSE, DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE); ok(r, "DuplicateHandle error %lu\n", GetLastError()); - todo_wine ok(f != out, "f == out\n"); + ok(f == out || broken(/* Win7 */ (((ULONG_PTR)f & 3) == 3) && (f != out)), "f != out\n"); CloseHandle(out); }
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=113104
Your paranoid android.
=== debian11 (32 bit Hindi:India report) ===
kernel32: process.c:1582: Test failed: GetLastError: expecting 87 got 6 process.c:1592: Test failed: GetLastError: expecting 87 got 6 process.c:1623: Test failed: Console:CursorX expected 4294955996, but got 0
V2: - fix broken results on Win7
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/kernel32/tests/process.c | 93 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index ebadc2e8d12..7b11e001537 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -336,7 +336,8 @@ static void doChild(const char* file, const char* option) HANDLE hFile = CreateFileA(file, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); HANDLE snapshot; PROCESSENTRY32 pe; - BOOL ret; + BOOL ret, bin, bout, berr; + DWORD flin, flout, flerr;
if (hFile == INVALID_HANDLE_VALUE) return;
@@ -451,6 +452,18 @@ static void doChild(const char* file, const char* option) childPrintf(hFile, "CurrDirW=%s\n", encodeW(bufW)); childPrintf(hFile, "\n");
+ bin = GetHandleInformation(GetStdHandle(STD_INPUT_HANDLE), &flin); + bout = GetHandleInformation(GetStdHandle(STD_OUTPUT_HANDLE), &flout); + berr = GetHandleInformation(GetStdHandle(STD_ERROR_HANDLE), &flerr); + if (bin || bout || berr) + { + childPrintf(hFile, "[StdHandleInfo]\n"); + if (bin ) childPrintf(hFile, "hStdInput=%lu\n", flin); + if (bout) childPrintf(hFile, "hStdOutput=%lu\n", flout); + if (berr) childPrintf(hFile, "hStdError=%lu\n", flerr); + childPrintf(hFile, "\n"); + } + if (option && strcmp(option, "console") == 0) { CONSOLE_SCREEN_BUFFER_INFO sbi; @@ -523,6 +536,15 @@ static void doChild(const char* file, const char* option) CloseHandle(hFile); }
+static BOOL isChildPresent( const char* sect, const char* key ) +{ + char buf[1024+4*MAX_LISTED_ENV_VAR]; + + GetPrivateProfileStringA(sect, key, "-", buf, sizeof(buf), resfile); + if (buf[0] == '\0' || (buf[0] == '-' && buf[1] == '\0')) return FALSE; + return TRUE; +} + static char* getChildString(const char* sect, const char* key) { char buf[1024+4*MAX_LISTED_ENV_VAR]; @@ -614,10 +636,17 @@ static void ok_child_int( int line, const char *sect, const char *key, UINT expe ok_(__FILE__, line)( result == expect, "%s:%s expected %u, but got %u\n", sect, key, expect, result ); }
+static void ok_child_intbrk( int line, const char *sect, const char *key, UINT expect, UINT brk ) +{ + UINT result = GetPrivateProfileIntA( sect, key, !expect, resfile ); + ok_(__FILE__, line)( result == expect || broken(result == brk), "%s:%s expected %u, but got %u\n", sect, key, expect, result ); +} + #define okChildString(sect, key, expect) ok_child_string(__LINE__, (sect), (key), (expect), 1 ) #define okChildIString(sect, key, expect) ok_child_string(__LINE__, (sect), (key), (expect), 0 ) #define okChildStringWA(sect, key, expect) ok_child_stringWA(__LINE__, (sect), (key), (expect), 1 ) #define okChildInt(sect, key, expect) ok_child_int(__LINE__, (sect), (key), (expect)) +#define okChildIntBroken(sect, key, expect, brk) ok_child_intbrk(__LINE__, (sect), (key), (expect), (brk))
static void test_Startup(void) { @@ -1691,6 +1720,68 @@ static void test_Console(void)
release_memory(); DeleteFileA(resfile); + + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = NULL; + sa.bInheritHandle = TRUE; + + /* test passing in startupinfo inheritable and non inheritable handles (CreateProcess not inheriting handles) */ + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + startup.dwFlags = STARTF_USESTDHANDLES; + startup.hStdInput = CreateFileA("NUL", GENERIC_READ|GENERIC_WRITE, 0, &sa, OPEN_EXISTING, 0, 0); + startup.hStdOutput = CreateFileA("NUL", GENERIC_READ|GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); + startup.hStdError = (HANDLE)(DWORD_PTR)0x00585840; + ok(startup.hStdInput != INVALID_HANDLE_VALUE, "failed to open NUL\n"); + ok(startup.hStdOutput != INVALID_HANDLE_VALUE, "failed to open NUL\n"); + sprintf(buffer, ""%s" process dump "%s"", selfname, resfile); + ok(CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info), "CreateProcess\n"); + wait_and_close_child_process(&info); + CloseHandle(startup.hStdInput); + + reload_child_info(resfile); + /* handles values have been zero:ed */ + todo_wine okChildIntBroken("StartupInfoA", "hStdInput", 0, /* Win7 */ (DWORD_PTR)startup.hStdInput); + todo_wine okChildIntBroken("StartupInfoA", "hStdOutput", 0, /* Win7 */ (DWORD_PTR)startup.hStdOutput); + todo_wine okChildIntBroken("StartupInfoA", "hStdError", 0, /* Win7 */ (DWORD_PTR)startup.hStdError); + todo_wine okChildIntBroken("StartupInfoW", "hStdInput", 0, /* Win7 */ (DWORD_PTR)startup.hStdInput); + todo_wine okChildIntBroken("StartupInfoW", "hStdOutput", 0, /* Win7 */ (DWORD_PTR)startup.hStdOutput); + todo_wine okChildIntBroken("StartupInfoW", "hStdError", 0, /* Win7 */ (DWORD_PTR)startup.hStdError); + /* even inheritable objects are not inherited */ + todo_wine ok(!isChildPresent("StdHandleInfo", "hStdInput"), "hStdInput shouldn't be present\n"); + todo_wine ok(!isChildPresent("StdHandleInfo", "hStdOutput"), "hStdOuput shouldn't be present\n"); + ok(!isChildPresent("StdHandleInfo", "hStdError"), "hStdError shouldn't be present\n"); + release_memory(); + DeleteFileA(resfile); + + /* test passing in startupinfo inheritable and non inheritable handles (CreateProcess inhering handles)) */ + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + startup.dwFlags = STARTF_USESTDHANDLES; + startup.hStdInput = CreateFileA("NUL", GENERIC_READ|GENERIC_WRITE, 0, &sa, OPEN_EXISTING, 0, 0); + startup.hStdOutput = CreateFileA("NUL", GENERIC_READ|GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); + startup.hStdError = (HANDLE)(DWORD_PTR)0x00585840; + ok(startup.hStdInput != INVALID_HANDLE_VALUE, "failed to open NUL\n"); + ok(startup.hStdOutput != INVALID_HANDLE_VALUE, "failed to open NUL\n"); + sprintf(buffer, ""%s" process dump "%s"", selfname, resfile); + ok(CreateProcessA(NULL, buffer, NULL, NULL, TRUE, 0, NULL, NULL, &startup, &info), "CreateProcess\n"); + wait_and_close_child_process(&info); + CloseHandle(startup.hStdInput); + + reload_child_info(resfile); + /* handles values are available */ + okChildInt("StartupInfoA", "hStdInput", (DWORD_PTR)startup.hStdInput); + okChildInt("StartupInfoA", "hStdOutput", (DWORD_PTR)startup.hStdOutput); + okChildInt("StartupInfoA", "hStdError", (DWORD_PTR)startup.hStdError); + okChildInt("StartupInfoW", "hStdInput", (DWORD_PTR)startup.hStdInput); + okChildInt("StartupInfoW", "hStdOutput", (DWORD_PTR)startup.hStdOutput); + okChildInt("StartupInfoW", "hStdError", (DWORD_PTR)startup.hStdError); + /* only inheritable objects are inherited */ + okChildInt("StdHandleInfo", "hStdInput", HANDLE_FLAG_INHERIT); + ok(!isChildPresent("StdHandleInfo", "hStdOutput"), "hStdOutput shouldn't be present\n"); + ok(!isChildPresent("StdHandleInfo", "hStdError"), "hStdError shouldn't be present\n"); + release_memory(); + DeleteFileA(resfile); }
static void test_ExitCode(void)
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=113107
Your paranoid android.
=== w10pro64 (64 bit report) ===
kernel32: process.c:5060: Test failed: got 0xc0000004
=== debian11 (32 bit Hindi:India report) ===
kernel32: process.c:1611: Test failed: GetLastError: expecting 87 got 6 process.c:1621: Test failed: GetLastError: expecting 87 got 6 process.c:1650: Test failed: Console:SizeX expected 2, but got 0 process.c:1651: Test failed: Console:SizeY expected 0, but got 1 process.c:1652: Test failed: Console:CursorX expected 3, but got 0 process.c:1653: Test failed: Console:CursorY expected 0, but got 1 process.c:1654: Test failed: Console:Attributes expected 4100, but got 0 process.c:1655: Test failed: Console:winLeft expected 19, but got 0 process.c:1656: Test failed: Console:winTop expected 4294963112, but got 0 process.c:1658: Test failed: Console:winBottom expected 11310, but got 0 process.c:1659: Test failed: Console:maxWinWidth expected 32764, but got 0 process.c:1660: Test failed: Console:maxWinHeight expected 11264, but got 0 process.c:1663: Test failed: Console:InputMode expected 6619251, but got 0 process.c:1664: Test failed: Console:OutputMode expected 7536754, but got 0
-
Eric Pouech -
Marvin