[PATCH 0/9] MR8865: Cmd engine rewrite (part XXXIII)

[eric pouech (＠epo)]

This MR contains a couple of simple fixes to cmd.exe.
(Also includes a rewrite of ECHO command for readability).

[PATCH 1/9] programs/cmd: Add tests about opening/closing parenthesis.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 programs/cmd/tests/test_builtins.cmd     | 6 ++++++
 programs/cmd/tests/test_builtins.cmd.exp | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index a57ca301b37..62f243b7e82 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -72,6 +72,7 @@ echo@tab@word@tab@@space@
 echo @tab@word
 echo  @tab@word
 echo@tab@@tab@word
+echo(3

echo ------------ Testing mixed echo modes ------------
 echo @echo on> mixedEchoModes.cmd
@@ -1582,6 +1583,11 @@ echo echo outside of block without closing bracket>> blockclosing.cmd
 cmd.exe /Q /C blockclosing.cmd
 echo %ERRORLEVEL% nested
 ::
+call :setError 666
+echo ^)> blockclosing.cmd
+cmd.exe /Q /C blockclosing.cmd
+echo %ERRORLEVEL% unmatched
+::
 del blockclosing.cmd
 echo --- case sensitivity with and without /i option
 if bar==BAR echo if does not default to case sensitivity
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 4a1a462079d..ed4fedee250 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -127,6 +127,7 @@ word
 @tab@word
 @space@@tab@word
 @tab@word
+@todo_wine@3
 ------------ Testing mixed echo modes ------------
 @echo on
 if 1==1 echo foo
@@ -1029,6 +1030,7 @@ with closing bracket
 255 two lines
 before both blocks
 255 nested
+@todo_wine@0 unmatched
 --- case sensitivity with and without /i option
 if seems to default to case sensitivity
 if /i seems to work

[PATCH 2/9] programs/cmd: Don't create binary node with NULL RHS.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 programs/cmd/wcmdmain.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index ca113bb97fc..7e061b3bc3c 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -2803,9 +2803,9 @@ static BOOL node_builder_parse(struct node_builder *builder, unsigned precedence
             {
                 ERROR_IF(tkn != TKN_EOL);
                 node_builder_consume(builder);
-                /* FIXME potential empty here?? */
                 ERROR_IF(!node_builder_parse(builder, 0, &right));
-                left = node_create_binary(CMD_CONCAT, left, right);
+                if (right)
+                    left = node_create_binary(CMD_CONCAT, left, right);
             }
             node_builder_consume(builder);
             /* if we had redirection before '(', add them up front */
@@ -2841,7 +2841,8 @@ static BOOL node_builder_parse(struct node_builder *builder, unsigned precedence
                     break;
                 }
                 ERROR_IF(!node_builder_parse(builder, token_get_precedence(tkn), &right));
-                left = node_create_binary(CMD_CONCAT, left, right);
+                if (right)
+                    left = node_create_binary(CMD_CONCAT, left, right);
             }
             break;
         case TKN_AMPAMP:

[PATCH 3/9] programs/cmd: Simplify builtin ECHO implementation.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 programs/cmd/builtins.c | 79 +++++++++++------------------------------
 1 file changed, 21 insertions(+), 58 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index 78ef855d099..a24396dc8bf 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -1554,73 +1554,36 @@ RETURN_CODE WCMD_delete(WCHAR *args)
     return errorlevel;
 }

-/*
- * WCMD_strtrim
- *
- * Returns a trimmed version of s with all leading and trailing whitespace removed
- * Pre: s non NULL
- *
- */
-static WCHAR *WCMD_strtrim(const WCHAR *s)
-{
-    DWORD len = lstrlenW(s);
-    const WCHAR *start = s;
-    WCHAR* result;
-
-    result = xalloc((len + 1) * sizeof(WCHAR));
-
-    while (iswspace(*start)) start++;
-    if (*start) {
-        const WCHAR *end = s + len - 1;
-        while (end > start && iswspace(*end)) end--;
-        memcpy(result, start, (end - start + 2) * sizeof(WCHAR));
-        result[end - start + 1] = '\0';
-    } else {
-        result[0] = '\0';
-    }
-
-    return result;
-}
-
 /****************************************************************************
  * WCMD_echo
  *
  * Echo input to the screen (or not). We don't try to emulate the bugs
  * in DOS (try typing "ECHO ON AGAIN" for an example).
  */
-
 RETURN_CODE WCMD_echo(const WCHAR *args)
 {
-  int count;
-  const WCHAR *origcommand = args;
-  WCHAR *trimmed;
-
-  if (   args[0]==' ' || args[0]=='\t' || args[0]=='.'
-      || args[0]==':' || args[0]==';'  || args[0]=='/')
-    args++;
-
-  trimmed = WCMD_strtrim(args);
-  if (!trimmed) return NO_ERROR;
-
-  count = lstrlenW(trimmed);
-  if (count == 0 && origcommand[0]!='.' && origcommand[0]!=':'
-                 && origcommand[0]!=';' && origcommand[0]!='/') {
-    if (echo_mode) WCMD_output(WCMD_LoadMessage(WCMD_ECHOPROMPT), L"ON");
-    else WCMD_output (WCMD_LoadMessage(WCMD_ECHOPROMPT), L"OFF");
-    free(trimmed);
+    const WCHAR *toskip = L".:;/";
+    const WCHAR *skipped = NULL;
+    WCHAR *trimmed;
+
+    if (iswspace(args[0]) || (args[0] && (skipped = wcschr(toskip, args[0])))) args++;
+
+    trimmed = WCMD_skip_leading_spaces((WCHAR *)args);
+
+    if (CompareStringW(LOCALE_USER_DEFAULT, NORM_IGNORECASE | SORT_STRINGSORT, trimmed, 2, L"ON", 2) == CSTR_EQUAL &&
+        *WCMD_skip_leading_spaces(trimmed + 2) == L'\0')
+        echo_mode = TRUE;
+    else if (CompareStringW(LOCALE_USER_DEFAULT, NORM_IGNORECASE | SORT_STRINGSORT, trimmed, 3, L"OFF", 3) == CSTR_EQUAL &&
+             *WCMD_skip_leading_spaces(trimmed + 3) == L'\0')
+        echo_mode = FALSE;
+    else if (!trimmed[0] && !skipped)
+        WCMD_output(WCMD_LoadMessage(WCMD_ECHOPROMPT), echo_mode ? L"ON" : L"OFF");
+    else
+    {
+        WCMD_output_asis(args);
+        WCMD_output_asis(L"\r\n");
+    }
     return NO_ERROR;
-  }
-
-  if (lstrcmpiW(trimmed, L"ON") == 0)
-    echo_mode = TRUE;
-  else if (lstrcmpiW(trimmed, L"OFF") == 0)
-    echo_mode = FALSE;
-  else {
-    WCMD_output_asis (args);
-    WCMD_output_asis(L"\r\n");
-  }
-  free(trimmed);
-  return NO_ERROR;
 }

/*****************************************************************************

[PATCH 4/9] programs/cmd: Fix handling of '(' in echo commands.

[Eric Pouech]

From: Eric Pouech

Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=57913

Signed-off-by: Eric Pouech 
---
 programs/cmd/builtins.c                  | 2 +-
 programs/cmd/tests/test_builtins.cmd.exp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index a24396dc8bf..1066ff2ea97 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -1562,7 +1562,7 @@ RETURN_CODE WCMD_delete(WCHAR *args)
  */
 RETURN_CODE WCMD_echo(const WCHAR *args)
 {
-    const WCHAR *toskip = L".:;/";
+    const WCHAR *toskip = L".:;/(";
     const WCHAR *skipped = NULL;
     WCHAR *trimmed;

diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index ed4fedee250..571f2da01e5 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -127,7 +127,7 @@ word
 @tab@word
 @space@@tab@word
 @tab@word
-@todo_wine@3
+3
 ------------ Testing mixed echo modes ------------
 @echo on
 if 1==1 echo foo

[PATCH 5/9] programs/cmd: Fix unmatched closing parenthesis handling.

[Eric Pouech]

From: Eric Pouech

Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=57912

Signed-off-by: Eric Pouech 
---
 programs/cmd/tests/test_builtins.cmd.exp | 2 +-
 programs/cmd/wcmdmain.c                  | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 571f2da01e5..a1d55c247fc 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -1030,7 +1030,7 @@ with closing bracket
 255 two lines
 before both blocks
 255 nested
-@todo_wine@0 unmatched
+0 unmatched
 --- case sensitivity with and without /i option
 if seems to default to case sensitivity
 if /i seems to work
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index 7e061b3bc3c..8f0d0d636b7 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -3553,6 +3553,9 @@ enum read_parse_line WCMD_ReadAndParseLine(CMD_NODE **output)
                                  curRedirs, &curRedirsLen,
                                  &curCopyTo, &curLen);
               node_builder_push_token(&builder, TKN_CLOSEPAR);
+          } else if (curStringLen == 0 && curCopyTo == curString) {
+              /* unmatched closing ')': silently skip rest of line */
+              curPos += wcslen(curPos) - 1;
           } else {
               curCopyTo[(*curLen)++] = *curPos;
           }

[PATCH 6/9] programs/cmd: Add more tests about ERASE builtin command.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 programs/cmd/tests/test_builtins.bat     | 1 +
 programs/cmd/tests/test_builtins.bat.exp | 1 +
 programs/cmd/tests/test_builtins.cmd     | 1 +
 programs/cmd/tests/test_builtins.cmd.exp | 1 +
 4 files changed, 4 insertions(+)

diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat
index 728a0e4eafc..05abc8de655 100644
--- a/programs/cmd/tests/test_builtins.bat
+++ b/programs/cmd/tests/test_builtins.bat
@@ -147,6 +147,7 @@ call :setError 666 & (erase &&echo SUCCESS !errorlevel!||echo FAILURE !errorleve
 call :setError 666 & (erase fileE &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
 call :setError 666 & (erase i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
 call :setError 666 & (erase file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
+call :setError 666 & (erase *.idontexistatall &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
 cd .. && rd /q /s foo

echo --- success/failure for change drive command
diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp
index a6b583f78c1..d1e4b565e67 100644
--- a/programs/cmd/tests/test_builtins.bat.exp
+++ b/programs/cmd/tests/test_builtins.bat.exp
@@ -94,6 +94,7 @@ FAILURE 1
 SUCCESS 0
 FAILURE 1
 FAILURE 1
+@todo_wine@SUCCESS 0
 --- success/failure for change drive command
 SUCCESS 666
 FAILURE 1
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 62f243b7e82..6a6b1196ad2 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -665,6 +665,7 @@ call :setError 666 & (erase &&echo SUCCESS !errorlevel!||echo FAILURE !errorleve
 call :setError 666 & (erase fileE &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
 call :setError 666 & (erase i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
 call :setError 666 & (erase file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
+call :setError 666 & (erase *.idontexistatall &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!)
 cd .. && rd /q /s foo

echo --- success/failure for change drive command
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index a1d55c247fc..179a989e76f 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -556,6 +556,7 @@ FAILURE 1
 SUCCESS 0
 FAILURE 1
 FAILURE 1
+@todo_wine@SUCCESS 0
 --- success/failure for change drive command
 SUCCESS 666
 FAILURE 1

[PATCH 7/9] programs/cmd: Fix return code for ERASE builtin command.

[Eric Pouech]

From: Eric Pouech

When run on a wild card:ed parameter without any match.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58027

Signed-off-by: Eric Pouech 
---
 programs/cmd/builtins.c                  | 1 +
 programs/cmd/tests/test_builtins.bat.exp | 2 +-
 programs/cmd/tests/test_builtins.cmd.exp | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index 1066ff2ea97..48c8e5e1097 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -1363,6 +1363,7 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) {
     hff = FindFirstFileW(argCopy, &fd);
     if (hff == INVALID_HANDLE_VALUE) {
       handleParm = FALSE;
+      found = wcschr(argCopy,'*') != NULL || wcschr(argCopy,'?') != NULL;
     } else {
       found = TRUE;
     }
diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp
index d1e4b565e67..608f3d11e15 100644
--- a/programs/cmd/tests/test_builtins.bat.exp
+++ b/programs/cmd/tests/test_builtins.bat.exp
@@ -94,7 +94,7 @@ FAILURE 1
 SUCCESS 0
 FAILURE 1
 FAILURE 1
-@todo_wine@SUCCESS 0
+SUCCESS 0
 --- success/failure for change drive command
 SUCCESS 666
 FAILURE 1
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 179a989e76f..1a1be21034e 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -556,7 +556,7 @@ FAILURE 1
 SUCCESS 0
 FAILURE 1
 FAILURE 1
-@todo_wine@SUCCESS 0
+SUCCESS 0
 --- success/failure for change drive command
 SUCCESS 666
 FAILURE 1

[PATCH 8/9] programs/cmd: Add tests about (not) echoing labels.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 programs/cmd/tests/test_builtins.cmd     | 8 ++++++++
 programs/cmd/tests/test_builtins.cmd.exp | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 6a6b1196ad2..1cc70caabed 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -130,6 +130,14 @@ type foo.txt | cmd.exe > NUL
 type foo.txt | cmd.exe > NUL
 @call :showEchoMode foo.tmp

+rem labels are not echoed (while all the other commands are)
+echo echo on > callme.cmd
+echo rem itsme >> callme.cmd
+echo :itsmeagain >> callme.cmd
+echo @echo off >> callme.cmd
+call callme.cmd
+@echo ---
+
 rem cleanup
 del foo.txt
 del callme.cmd
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 1a1be21034e..d70d751669e 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -150,6 +150,9 @@ ECHO_IS_ON
 ECHO_IS_OFF
 ECHO_IS_ON
 ECHO_IS_OFF
+
+@pwd@>rem itsme@space@@space@
+@todo_wine@---
 ------------ Testing parameterization ------------
 'a', 'b', 'c'
 '"a b c"', '', ''

[PATCH 9/9] programs/cmd: Don't display labels when echo mode is ON.

[Eric Pouech]

From: Eric Pouech

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58614

Signed-off-by: Eric Pouech 
---
 programs/cmd/tests/test_builtins.cmd     | 1 -
 programs/cmd/tests/test_builtins.cmd.exp | 1 -
 programs/cmd/wcmdmain.c                  | 2 +-
 3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 1cc70caabed..9f144bb4e0a 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -136,7 +136,6 @@ echo rem itsme >> callme.cmd
 echo :itsmeagain >> callme.cmd
 echo @echo off >> callme.cmd
 call callme.cmd
-@echo ---

rem cleanup
 del foo.txt
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index d70d751669e..6ec30eba3a7 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -152,7 +152,6 @@ ECHO_IS_ON
 ECHO_IS_OFF

@pwd@>rem itsme@space@@space@
-@todo_wine@---
 ------------ Testing parameterization ------------
 'a', 'b', 'c'
 '"a b c"', '', ''
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index 8f0d0d636b7..26c2be2e1c3 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -3197,7 +3197,7 @@ static WCHAR *fetch_next_line(BOOL first_line, WCHAR* buffer)

buffer = WCMD_skip_leading_spaces(buffer);
     /* Show prompt before batch line IF echo is on and in batch program */
-    if (WCMD_is_in_context(NULL) && echo_mode && *buffer && *buffer != '@')
+    if (WCMD_is_in_context(NULL) && echo_mode && *buffer && *buffer != L'@' && *buffer != L':')
     {
         if (first_line)
         {