[PATCH v5 0/3] MR8920: cmd: Fix some TYPE command behavior (one more try).
Giving this one final shot. If this is not deemed acceptable, I will abandon the effort and just move on to other things, no hard feelings. This is IMO a cleaner, more conservative/less invasive change. What is fixed: - Bug #56381, "TYPE c:\windows\winhelp.exe >foo", i.e. binary mode operation. I would probably consider this the main reason for this change. I'm trying to get the compiler mentioned in the bug report working. - Ctrl-Z termination of TYPE output to the console. - "TYPE con >foo", with Ctrl-Z handling, functionally equivalent to "COPY con foo". Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56381 -- v5: cmd: Fix TYPE behavior (now uses WCMD_copy_loop). cmd: Refactor WCMD_copy_loop out of WCMD_ManualCopy, and stop copy loop at EOF for /a mode. cmd/tests: Add test to check for TYPE truncation in binary mode. https://gitlab.winehq.org/wine/wine/-/merge_requests/8920
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/tests/test_builtins.cmd | 32 ++++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 4 +++ 2 files changed, 36 insertions(+) diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 9f144bb4e0a..4444333e79a 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1405,6 +1405,34 @@ echo ---6 type foobaw echo ---7 del foobaz foobay foobax foobaw +echo ---8 +rem generating sequence ab<ctrl-Z>c\n\r +echo 61621A630D0A>seq.hex +certutil -decodehex seq.hex seq.bin > nul +type seq.bin > foo0 +call :CompareFileSizes seq.bin foo0 +erase seq.hex seq.bin foo0 +echo ---9 +echo 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F >seq.hex +echo 10 11 12 13 14 15 16 17 18 19 1A 1B 1C 1D 1E 1F >>seq.hex +echo 20 21 22 23 24 25 26 27 28 29 2A 2B 2C 2D 2E 2F >>seq.hex +echo 30 31 32 33 34 35 36 37 38 39 3A 3B 3C 3D 3E 3F >>seq.hex +echo 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F >>seq.hex +echo 50 51 52 53 54 55 56 57 58 59 5A 5B 5C 5D 5E 5F >>seq.hex +echo 60 61 62 63 64 65 66 67 68 69 6A 6B 6C 6D 6E 6F >>seq.hex +echo 70 71 72 73 74 75 76 77 78 79 7A 7B 7C 7D 7E 7F >>seq.hex +echo 80 81 82 83 84 85 86 87 88 89 8A 8B 8C 8D 8E 8F >>seq.hex +echo 90 91 92 93 94 95 96 97 98 99 9A 9B 9C 9D 9E 9F >>seq.hex +echo A0 A1 A2 A3 A4 A5 A6 A7 A8 A9 AA AB AC AD AE AF >>seq.hex +echo B0 B1 B2 B3 B4 B5 B6 B7 B8 B9 BA BB BC BD BE BF >>seq.hex +echo C0 C1 C2 C3 C4 C5 C6 C7 C8 C9 CA CB CC CD CE CF >>seq.hex +echo D0 D1 D2 D3 D4 D5 D6 D7 D8 D9 DA DB DC DD DE DF >>seq.hex +echo E0 E1 E2 E3 E4 E5 E6 E7 E8 E9 EA EB EC ED EE EF >>seq.hex +echo F0 F1 F2 F3 F4 F5 F6 F7 F8 F9 FA FB FC FD FE FF >>seq.hex +certutil -decodehex seq.hex seq.bin > nul +type seq.bin > foo0 +call :CompareFileSizes seq.bin foo0 +erase seq.hex seq.bin foo0 echo ------------ Testing NUL ------------ md foobar & cd foobar @@ -3513,6 +3541,10 @@ shift if not "%1"=="" goto :CheckFileSize goto :eof +:CompareFileSizes +if "%~z1"=="%~z2" (echo passed) else (echo failed) +goto :eof + :testcopy rem ----------------------- diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 6ec30eba3a7..e465ad3f8bc 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -994,6 +994,10 @@ foobay ---7 +---8 +(a)todo_wine@passed +---9 +(a)todo_wine@passed ------------ Testing NUL ------------ bar bar -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8920
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/builtins.c | 71 +++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index dd9808aa9f2..8425781d664 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -540,6 +540,49 @@ end: return ret; } +/**************************************************************************** + * WCMD_copy_loop + * + * Copies from a file + * optionally reading only until EOF (ascii copy) + * Returns TRUE on success + */ +static BOOL WCMD_copy_loop(HANDLE in, HANDLE out, BOOL ascii) +{ + BOOL ok; + DWORD bytesread, byteswritten; + char *eof = NULL; + + /* Loop copying data from source to destination until EOF read */ + do + { + char buffer[MAXSTRING]; + + ok = ReadFile(in, buffer, MAXSTRING, &bytesread, NULL); + if (ok) { + + /* Stop at first EOF */ + if (ascii) { + eof = (char *)memchr((void *)buffer, '\x1a', bytesread); + if (eof) bytesread = (eof - buffer); + } + + if (bytesread) { + ok = WriteFile(out, buffer, bytesread, &byteswritten, NULL); + if (!ok || byteswritten != bytesread) { + WINE_ERR("Unexpected failure writing, rc=%ld\n", + GetLastError()); + } + } + } else { + WINE_ERR("Unexpected failure reading, rc=%ld\n", + GetLastError()); + } + } while (ok && bytesread > 0 && !eof); + + return ok; +} + /**************************************************************************** * WCMD_ManualCopy * @@ -552,7 +595,6 @@ static BOOL WCMD_ManualCopy(WCHAR *srcname, WCHAR *dstname, BOOL ascii, BOOL app { HANDLE in,out; BOOL ok; - DWORD bytesread, byteswritten; WINE_TRACE("Manual Copying %s to %s (ascii: %u) (append: %u)\n", wine_dbgstr_w(srcname), wine_dbgstr_w(dstname), ascii, append); @@ -578,32 +620,7 @@ static BOOL WCMD_ManualCopy(WCHAR *srcname, WCHAR *dstname, BOOL ascii, BOOL app SetFilePointer(out, 0, NULL, FILE_END); } - /* Loop copying data from source to destination until EOF read */ - do - { - char buffer[MAXSTRING]; - - ok = ReadFile(in, buffer, MAXSTRING, &bytesread, NULL); - if (ok) { - - /* Stop at first EOF */ - if (ascii) { - char *ptr = (char *)memchr((void *)buffer, '\x1a', bytesread); - if (ptr) bytesread = (ptr - buffer); - } - - if (bytesread) { - ok = WriteFile(out, buffer, bytesread, &byteswritten, NULL); - if (!ok || byteswritten != bytesread) { - WINE_ERR("Unexpected failure writing to %s, rc=%ld\n", - wine_dbgstr_w(dstname), GetLastError()); - } - } - } else { - WINE_ERR("Unexpected failure reading from %s, rc=%ld\n", - wine_dbgstr_w(srcname), GetLastError()); - } - } while (ok && bytesread > 0); + ok = WCMD_copy_loop(in, out, ascii); CloseHandle(out); CloseHandle(in); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8920
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/builtins.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 8425781d664..ff835386560 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -3433,31 +3433,30 @@ RETURN_CODE WCMD_type(WCHAR *args) while (argN) { WCHAR *thisArg = WCMD_parameter (args, argno++, &argN, FALSE, FALSE); - HANDLE h; - WCHAR buffer[512]; - DWORD count; + HANDLE hIn, hOut; + DWORD console_mode; if (!argN) break; WINE_TRACE("type: Processing arg '%s'\n", wine_dbgstr_w(thisArg)); - h = CreateFileW(thisArg, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, NULL); - if (h == INVALID_HANDLE_VALUE) { + hIn = CreateFileW(thisArg, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + if (hIn == INVALID_HANDLE_VALUE) { WCMD_print_error (); WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); return errorlevel = ERROR_INVALID_FUNCTION; - } else { - if (writeHeaders) { - WCMD_output_stderr(L"\n%1\n\n\n", thisArg); - } - while (WCMD_ReadFile(h, buffer, ARRAY_SIZE(buffer) - 1, &count)) { - if (count == 0) break; /* ReadFile reports success on EOF! */ - buffer[count] = 0; - WCMD_output_asis (buffer); - } - CloseHandle (h); } + hOut = GetStdHandle(STD_OUTPUT_HANDLE); + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", thisArg); + } + + WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + + CloseHandle (hIn); } + return errorlevel = return_code; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8920
On Thu Sep 11 07:33:07 2025 +0000, eric pouech wrote:
Code before my change truncated at NUL, not Ctrl-Z. winhelp.exe test was the specific case mentioned in the bug and I added it as a regression test. Your test does not have a case that checks for truncation at NUL. I also had difficulty creating a file containing a Ctrl-Z character from the .cmd file, which is what prompted me to create the file in batch.c. My test file had an occurrence of each ASCII character, to ensure that there would be not truncation at any character. fair point. feel free to extend tests with such cases (NUL is definitively a good candidate) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8920#note_115656
I'm not sure it's worth it trying all possible input character as: - ASCII characters (20-7F) bring nothing - it could have some unwanted side effects depending on input locale for upper half of the values - tests should be tests, not tools to list all possible values (it also consumes test time) so I'd suggest that the second test duplicates the first one, but replacing ctrl-z by EOF=0 (so that we test unitary the behavior of the control characters that should end a file in text mode) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8920#note_115819
participants (3)
-
eric pouech (＠epo) -
Joe Souza
-
Joe Souza (＠JoeS209)