[PATCH v6 0/1] MR8137: kernel32: Implement ReadConsoleInputEx
This implements the method only with empty flags, nowait (0x2) and all flags on noremove(0x1) | nowait(0x2) -- v6: kernel32: Implement ReadConsoleInputEx https://gitlab.winehq.org/wine/wine/-/merge_requests/8137
From: Jayson Reis <santosdosreis(a)gmail.com> This implements the method only with empty flags, nowait (0x2) and all flags on noremove(0x1) | nowait(0x2) --- dlls/kernel32/kernel32.spec | 4 +- dlls/kernel32/tests/console.c | 54 ++++++++++++++++++++++++++ dlls/kernelbase/console.c | 67 +++++++++++++++++++++++++++++++++ dlls/kernelbase/kernelbase.spec | 4 +- 4 files changed, 125 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index caa6c92b653..7d008cbaccd 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -1218,8 +1218,8 @@ @ stdcall -import RaiseFailFastException(ptr ptr long) @ stdcall -import ReadConsoleA(long ptr long ptr ptr) @ stdcall -import ReadConsoleInputA(long ptr long ptr) -@ stub ReadConsoleInputExA -@ stub ReadConsoleInputExW +@ stdcall -import ReadConsoleInputExA(long ptr long ptr long) +@ stdcall -import ReadConsoleInputExW(long ptr long ptr long) @ stdcall -import ReadConsoleInputW(long ptr long ptr) @ stdcall -import ReadConsoleOutputA(long ptr long long ptr) @ stdcall -import ReadConsoleOutputAttribute(long ptr long long ptr) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 398b818776a..6578aa0692d 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -35,6 +35,8 @@ static DWORD (WINAPI *pGetConsoleProcessList)(LPDWORD, DWORD); static HANDLE (WINAPI *pOpenConsoleW)(LPCWSTR,DWORD,BOOL,DWORD); static BOOL (WINAPI *pSetConsoleInputExeNameA)(LPCSTR); static BOOL (WINAPI *pVerifyConsoleIoHandle)(HANDLE handle); +static BOOL (WINAPI *pReadConsoleInputExA)(HANDLE handle, INPUT_RECORD *buffer, + DWORD length, DWORD *count, USHORT flags); static BOOL skip_nt; @@ -81,6 +83,7 @@ static void init_function_pointers(void) KERNEL32_GET_PROC(OpenConsoleW); KERNEL32_GET_PROC(SetConsoleInputExeNameA); KERNEL32_GET_PROC(VerifyConsoleIoHandle); + KERNEL32_GET_PROC(ReadConsoleInputExA); #undef KERNEL32_GET_PROC } @@ -3426,6 +3429,55 @@ static void test_ReadConsole(HANDLE input) CloseHandle(output); } +static void test_ReadConsoleInputEx(HANDLE input) +{ + DWORD ret, count; + INPUT_RECORD buf; + HANDLE invalid_handle = (HANDLE)-1; + + ret = pReadConsoleInputExA(input, &buf, 1, &count, 0x0001); + if (!ret && GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) { + skip("ReadConsoleInputExA not implemented\n"); + } + + // When empty, just return as is without blocking + ret = pReadConsoleInputExA(input, &buf, 1, &count, 0x0002); + ok(ret, "ReadConsoleInputExA nowait failed %ld %ld \n", count, GetLastError()); + + memset(&buf, 0, sizeof(buf)); + buf.EventType = MOUSE_EVENT; + buf.Event.MouseEvent.dwEventFlags = DOUBLE_CLICK; + ret = WriteConsoleInputA(input, &buf, 1, &count); + ok(ret, "got error %lu\n", GetLastError()); + + SetLastError(0); + memset(&buf, 0, sizeof(buf)); + ret = pReadConsoleInputExA(input, &buf, 1, &count, 0x0000); + ok(ret && buf.EventType == MOUSE_EVENT, "ReadConsoleInputExA failed %#lx \n", GetLastError()); + + memset(&buf, 0, sizeof(buf)); + buf.EventType = MOUSE_EVENT; + buf.Event.MouseEvent.dwEventFlags = DOUBLE_CLICK; + ret = WriteConsoleInputA(input, &buf, 1, &count); + ok(ret, "got error %lu\n", GetLastError()); + + memset(&buf, 0, sizeof(buf)); + ret = pReadConsoleInputExA(input, &buf, 1, &count, 0x0002); + ok(ret && buf.EventType == MOUSE_EVENT, "ReadConsoleInputExA failed %#lx.\n", GetLastError()); + + /* Invalid parameter */ + memset(&buf, 0, sizeof(buf)); + ret = pReadConsoleInputExA(input, &buf, 1, &count, 0xFFFF); + ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, + "ReadConsoleInputExA should be invalid parameter %#lx.\n", GetLastError()); + + /* Invalid handle */ + memset(&buf, 0, sizeof(buf)); + ret = pReadConsoleInputExA(invalid_handle, &buf, 1, &count, 0xFFFF); + ok(!ret && GetLastError() == ERROR_INVALID_HANDLE, + "ReadConsoleInputExA should be invalid handle %#lx.\n", GetLastError()); +} + static void test_GetCurrentConsoleFont(HANDLE std_output) { BOOL ret; @@ -5826,6 +5878,8 @@ START_TEST(console) if (!ret) return; test_ReadConsole(hConIn); + test_ReadConsoleInputEx(hConIn); + /* Non interactive tests */ testCursor(hConOut, sbi.dwSize); /* test parameters (FIXME: test functionality) */ diff --git a/dlls/kernelbase/console.c b/dlls/kernelbase/console.c index 8c90eb321df..1996361c69a 100644 --- a/dlls/kernelbase/console.c +++ b/dlls/kernelbase/console.c @@ -1821,6 +1821,73 @@ BOOL WINAPI ReadConsoleInputW( HANDLE handle, INPUT_RECORD *buffer, DWORD length } + +/*********************************************************************** + * ReadConsoleInputExW (kernelbase.@) + */ +BOOL WINAPI ReadConsoleInputExW( HANDLE handle, INPUT_RECORD *buffer, DWORD length, DWORD *count, USHORT flags ) +{ + USHORT no_remove = 1; + USHORT no_wait = 2; + USHORT all = 3; + + TRACE( "(%p,%p,%ld,%p,%#x).\n", handle, buffer, length, count, flags ); + + if (!DeviceIoControl( handle, IOCTL_CONDRV_READ_CONSOLE, NULL, 0, + NULL, 0, NULL, NULL )) + { + SetLastError( ERROR_INVALID_HANDLE ); + return FALSE; + } + + if (flags > all) + { + SetLastError(ERROR_INVALID_PARAMETER); + return FALSE; + } + + /* if all flags are set, it is the same as calling PeekConsoleInputW */ + if ((flags & all) == all) + return PeekConsoleInputW( handle, buffer, length, count) ; + + if (flags == no_wait) { + /* TODO: Does it need to be the exact count that the user supplied? */ + if (PeekConsoleInputW( handle, buffer, 1, count) && *count > 0) { + return ReadConsoleInputExW( handle, buffer, length, count, 0 ); + } + return TRUE; + } + + if (flags == no_remove) { + FIXME("ReadConsoleInputExW not implemented for NOREMOVE"); + SetLastError( ERROR_CALL_NOT_IMPLEMENTED ); + return FALSE; + } + + if (!console_ioctl( handle, IOCTL_CONDRV_READ_INPUT, NULL, 0, + buffer, length * sizeof(*buffer), count )) + return FALSE; + *count /= sizeof(*buffer); + return TRUE; +} + + + +/*********************************************************************** + * ReadConsoleInputExA (kernelbase.@) + */ +BOOL WINAPI ReadConsoleInputExA( HANDLE handle, INPUT_RECORD *buffer, DWORD length, DWORD *count, USHORT flags ) +{ + DWORD read; + + if (!ReadConsoleInputExW( handle, buffer, length, &read, flags )) return FALSE; + input_records_WtoA( buffer, read ); + if (count) *count = read; + return TRUE; +} + + + /****************************************************************************** * WriteConsoleInputA (kernelbase.@) */ diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec index fd9868fb69e..45ee0e340f3 100644 --- a/dlls/kernelbase/kernelbase.spec +++ b/dlls/kernelbase/kernelbase.spec @@ -1280,8 +1280,8 @@ @ stdcall ReOpenFile(ptr long long long) @ stdcall ReadConsoleA(long ptr long ptr ptr) @ stdcall ReadConsoleInputA(long ptr long ptr) -@ stub ReadConsoleInputExA -@ stub ReadConsoleInputExW +@ stdcall ReadConsoleInputExA(long ptr long ptr long) +@ stdcall ReadConsoleInputExW(long ptr long ptr long) @ stdcall ReadConsoleInputW(long ptr long ptr) @ stdcall ReadConsoleOutputA(long ptr long long ptr) @ stdcall ReadConsoleOutputAttribute(long ptr long long ptr) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8137
On Sun May 25 13:31:25 2025 +0000, Alfred Agrell wrote:
You can call a function without public header, just move that prototype to console.c (either near the top, or just before ExA - I don't know what Wine code style says about that). The race happens if two threads read from that console simultaneously, both with nowait; Peek returns for both, thread 1 calls Read, and thread 2 calls Read too and blocks because it's empty. Tests should be run on Windows, yes. That's how we discover which behavior to implement in Wine. We usually SetLastError(0xdeadbeef) before calling something that should (or shouldn't) set the last error, to make sure we're not seeing a leftover error from a previous operation. If PeekW returns false, then you should return false too, not think you're in the *count=0 branch and return true. The code style issue is probably the // comments, we use only /* in Wine. (Also it's spelled that, not tha.) Actually if it's an exported API it's OK to declare it in a public header.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8137#note_104479
@julliard It's not present in native export .LIB. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8137#note_104533
On Mon May 26 07:25:12 2025 +0000, eric pouech wrote:
a couple of comments: * please read and conform to [Wine Coding style](https://gitlab.winehq.org/wine/wine/-/wikis/Wine-Developer's-Guide/Coding-Practice#some-notes-about-style) * AFAICT ReadConsoleInputEx\[AW\] isn't defined in Windows SDK so header files shouldn't be changed * tests must reflect what Windows does, not what your implementation does (hence the generated failures in CI pipeline) * this should be implemented using proper (new or modified) IOCTL:s; there's no way this API can be implemented on top of kernel32's one without being racy (as your proposal is). That's the reason why MS introduced this new API. Hi @epo I did a few changes but, I still cannot figure out why the tests on Windows return invalid handle but when I locally build the tests and copy kernel32_tests.exe to a Windows 11 and Windows 10 machine and run kernel32_tests.exe both work, any idea if there is something different I should do?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8137#note_104539
On Mon May 26 07:25:12 2025 +0000, Jayson Reis wrote:
Hi @epo I did a few changes but, I still cannot figure out why the tests on Windows return invalid handle but when I locally build the tests and copy kernel32_tests.exe to a Windows 11 and Windows 10 machine and run kernel32_tests.exe both work, any idea if there is something different I should do? not clear which 'tests on Windows' you're referring to.
what I see in pipeline result are failures at lines 3456 and 3466, but these failures aren't clearly an invalid handle -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8137#note_104540
participants (4)
-
Alexandre Julliard (＠julliard) -
eric pouech (＠epo)
-
Jayson Reis
-
Jayson Reis (＠jaysonsantos)