[PATCH 0/1] MR6605: cmd: Call ReadConsoleW() only with standard input handles.
From: Hans Leidekker <hans(a)codeweavers.com> --- programs/cmd/batch.c | 20 ++++++++++---------- programs/cmd/wcmdmain.c | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 25281df15f0..288f96a2eff 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -222,7 +222,16 @@ WCHAR *WCMD_fgets(WCHAR *buf, DWORD noChars, HANDLE h) /* We can't use the native f* functions because of the filename syntax differences between DOS and Unix. Also need to lose the LF (or CRLF) from the line. */ - if (!ReadConsoleW(h, buf, noChars, &charsRead, NULL)) { + if (h == GetStdHandle(STD_INPUT_HANDLE) && ReadConsoleW(h, buf, noChars, &charsRead, NULL) && charsRead) { + if (!charsRead) return NULL; + + /* Find first EOL */ + for (i = 0; i < charsRead; i++) { + if (buf[i] == '\n' || buf[i] == '\r') + break; + } + } + else { LARGE_INTEGER filepos; char *bufA; UINT cp; @@ -254,15 +263,6 @@ WCHAR *WCMD_fgets(WCHAR *buf, DWORD noChars, HANDLE h) i = MultiByteToWideChar(cp, 0, bufA, p - bufA, buf, noChars); free(bufA); } - else { - if (!charsRead) return NULL; - - /* Find first EOL */ - for (i = 0; i < charsRead; i++) { - if (buf[i] == '\n' || buf[i] == '\r') - break; - } - } /* Truncate at EOL (or end of buffer) */ if (i == noChars) diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 27c7def7900..4975c237dad 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -222,7 +222,7 @@ BOOL WCMD_ReadFile(const HANDLE hIn, WCHAR *intoBuf, const DWORD maxChars, LPDWO char *buffer; /* Try to read from console as Unicode */ - if (ReadConsoleW(hIn, intoBuf, maxChars, charsRead, NULL)) return TRUE; + if (hIn == GetStdHandle(STD_INPUT_HANDLE) && ReadConsoleW(hIn, intoBuf, maxChars, charsRead, NULL)) return TRUE; /* We assume it's a file handle and read then convert from assumed OEM codepage */ if (!(buffer = get_file_buffer())) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6605
why do need this? (and since we don't control what's the standard input handle is, I'm not sure the test against std handle is what you may want) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6605#note_83860
I have an application that hooks kernel functions and the ReadConsoleW() hook is not expected to be called (it's a stub). cmd.exe is passed a batch file on the command line (/C) so we're reading a regular file, not standard input. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6605#note_83861
In that case, likely GetFileType(h) == FILE_TYPE_CHAR would be a clearer discrimination to call into console APIs -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6605#note_83880
WCMD_fgets()/WCMD_ReadFile() are called with either a handle returned from CreateFileW() or GetStdHandle(STD_INPUT_HANDLE). So using h == GetStdHandle(STD_INPUT_HANDLE) seems clear and consistent to me. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6605#note_83885
On Wed Oct 2 12:22:02 2024 +0000, eric pouech wrote:
In that case, likely GetFileType(h) == FILE_TYPE_CHAR would be a clearer discrimination to call into console APIs I don't know the details here (so ignore my comment if it doesn't make sense) but we had to switch to `VerifyConsoleIoHandle()` to properly detect console in msvcrt.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6605#note_83886
On Wed Oct 2 12:22:02 2024 +0000, Piotr Caban wrote:
I don't know the details here (so ignore my comment if it doesn't make sense) but we had to switch to `VerifyConsoleIoHandle()` to properly detect console in msvcrt. I don't know the details of that fix but note that with this patch we still fall back to ReadFile() if ReadConsoleW() fails, e.g. because stdin was redirected.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6605#note_83887
the std input handle can be anything: - it's by default the input handle from which cmd.exe has been created (someone could invoke cmd.exe with "cmd /c foo.bat < COM1" and you get a serial line handle) - and it's changed with any redirection (so < COM1, < NUL, < CON would set a CHAR file type); and perhaps tomorrow | (piping) commands will be implemented with (named) pipes Piotr's suggestion is even cleaner (as it will not accept other CHAR devices) Note: we need to keep ReadConsole (instead of ReadFile) when we implement completion of commands -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6605#note_83889
participants (4)
-
eric pouech (＠epo) -
Hans Leidekker
-
Hans Leidekker (＠hans)
-
Piotr Caban (＠piotr)