[PATCH v7 0/4] MR8920: cmd: Fix some TYPE command behavior (one more try).

[Joe Souza (＠JoeS209)]

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

[PATCH v7 1/4] cmd/tests: Add test to check for TYPE truncation in binary mode.

[Joe Souza]

From: Joe Souza

---
 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
+@todo_wine@passed
+---9
+@todo_wine@passed
 ------------ Testing NUL ------------
 bar
 bar

[PATCH v7 2/4] cmd: Refactor WCMD_copy_loop out of WCMD_ManualCopy, and stop copy loop at EOF for /a mode.

[Joe Souza]

From: Joe Souza

---
 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);

[PATCH v7 3/4] cmd: Fix TYPE behavior (now uses WCMD_copy_loop).

[Joe Souza]

From: Joe Souza

---
 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;
 }

[PATCH v7 4/4] cmd/tests: Remove todo_wine from two TYPE tests that should pass after certutil changes are merged.

[Joe Souza]

From: Joe Souza

---
 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
-@todo_wine@passed
+passed
 ---9
-@todo_wine@passed
+passed
 ------------ Testing NUL ------------
 bar
 bar

[eric pouech (＠epo)]

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

[Joe Souza (＠JoeS209)]

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.

[Joe Souza (＠JoeS209)]

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.