[PATCH v7 0/4] 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 -- v7: cmd/tests: Remove todo_wine from two TYPE tests that should pass after certutil changes are merged. https://gitlab.winehq.org/wine/wine/-/merge_requests/8920
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/tests/test_builtins.cmd | 18 ++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 9f144bb4e0a..bf73af845bf 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1405,6 +1405,20 @@ 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 +rem generating sequence ab<NUL>c\n\r +echo 616200630D0A>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 +3527,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
From: Joe Souza <jsouza(a)yahoo.com> --- programs/cmd/tests/test_builtins.cmd.exp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index e465ad3f8bc..f9bec6813d0 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -995,9 +995,9 @@ foobay ---7 ---8 -(a)todo_wine@passed +passed ---9 -(a)todo_wine@passed +passed ------------ Testing NUL ------------ bar bar -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8920
LGTM some last formal items to fix: - in the title each commit, please use 'programs/cmd:' (instead of cmd:) as prefix to demark from dlls/ directory - third and fourth patch shall be merged (remember: we want at each commit that tests don't generate errors) - third commit should have a 'Wine-Bug:' line referring to the now fixed bug -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8920#note_116192
On Thu Sep 18 06:41:43 2025 +0000, eric pouech wrote:
LGTM some last formal items to fix: - in the title each commit, please use 'programs/cmd:' (instead of cmd:) as prefix to demark from dlls/ directory - third and fourth patch shall be merged (remember: we want at each commit that tests don't generate errors) - third commit should have a 'Wine-Bug:' line referring to the now fixed bug In other examples I've seen, "Wine-Bug" is in MR description, not commit description. Will do the rest soon.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8920#note_116194
On Thu Sep 18 06:41:43 2025 +0000, Joe Souza wrote:
In other examples I've seen, "Wine-Bug" is in MR description, not commit description. Will do the rest soon. Also, seems that every commit for cmd contains "cmd:", not "programs/cmd:". I think Alexandre removes the "programs/" if found.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8920#note_116195
participants (3)
-
eric pouech (＠epo) -
Joe Souza
-
Joe Souza (＠JoeS209)