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".
-- v4: 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.
From: Joe Souza jsouza@yahoo.com
--- programs/cmd/tests/test_builtins.cmd | 11 +++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 2 ++ 2 files changed, 13 insertions(+)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 9f144bb4e0a..6842733f30e 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1405,6 +1405,13 @@ 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 ------------ Testing NUL ------------ md foobar & cd foobar @@ -3513,6 +3520,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..0282a5dc566 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -994,6 +994,8 @@ foobay
---7 +---8 +@todo_wine@passed ------------ Testing NUL ------------ bar bar
From: Joe Souza jsouza@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);
From: Joe Souza jsouza@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; }
On Wed Sep 10 16:11:21 2025 +0000, eric pouech wrote:
thanks, that looks way better. Notes:
- I submitted MR!8947 to support -decodehex in certutil, so I'd rather
wait for this MR to be approved before approving this one so that we can make use of certutil,
- (and you'll have to remove the todo_wine after certutil is changed)
- last point: it would be better to split the second commit in two: 1)
the introduction of WCMD_copy_loop (explaining that it does fix COPY /a in some cases), b) the fix for TYPE command making use of WCMD_copy_loop. This would ensure that in the future, if someone does a git bisect, each single commit does a single thing.
- and the commit fixing the TYPE command shall have a Wine-Bug line
referring to the fixed bugzilla entry. TIA
Done.
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)
-
eric pouech (＠epo) -
Joe Souza
-
Joe Souza (＠JoeS209)