[reactos] 15/48: [CMD] Fixes for Batch error execution control flow. CORE-13713 CORE-13736

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=6eb1cae34819c275e0122…

commit 6eb1cae34819c275e01229d524a765e3340cdc5a
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Mon May 18 02:05:53 2020 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Wed Aug 19 20:35:58 2020 +0200

    [CMD] Fixes for Batch error execution control flow.
    CORE-13713 CORE-13736
    
    - In case execution of all batch contexts is stopped (by selecting "All"
      at the Ctrl-C/Ctrl-Break prompt), notify as well the CheckCtrlBreak()
      signal handler once there are no more batch contexts (this in effect
      resets the internal 'bLeaveAll' static flag in CheckCtrlBreak).
      This is an adaptation of the fix present in FreeCOM 1.5, first
      described in https://gcfl.net/FreeDOS/command.com/bugs074g.html .
    
    - Introduce a ParseErrorEx() helper that sets the 'bParseError' flag and
      displays a customized syntax-error message, only for the first syntax
      error encountered. Implement ParseError() around the *Ex function.
    
    - In batch mode, echo the original pre-parsed batch file line if a parse
      error has been encountered.
    
    - When running a compound command - including IF, FOR, command blocks -,
      and that control flow is modified by any CALL/GOTO/EXIT command,
      detect this while running the compound command so as to stop it and go
      back to the main batch execution loop, that will then set up the actual
      new command to run.
    
    - In GOTO, do not process any more parts of a compound command only when
      we have found a valid label.
---
 base/shell/cmd/batch.c    | 39 ++++++++++++++++++++++++++++++++++++---
 base/shell/cmd/batch.h    |  2 +-
 base/shell/cmd/cmd.c      | 22 ++++++++++++++++++----
 base/shell/cmd/cmd.h      | 12 ++++++++----
 base/shell/cmd/for.c      | 43 ++++++++++++++++++++++++++++++-------------
 base/shell/cmd/goto.c     | 10 +++++++++-
 base/shell/cmd/if.c       |  4 +---
 base/shell/cmd/internal.c |  5 +++++
 base/shell/cmd/parser.c   | 15 +++++++++++----
 9 files changed, 119 insertions(+), 33 deletions(-)

diff --git a/base/shell/cmd/batch.c b/base/shell/cmd/batch.c
index 6de2e657329..a5c31aece09 100644
--- a/base/shell/cmd/batch.c
+++ b/base/shell/cmd/batch.c
@@ -195,6 +195,24 @@ VOID ExitBatch(VOID)
         cmd_endlocal(_T(""));
 
     bc = bc->prev;
+
+#if 0
+    /* Do not process any more parts of a compound command */
+    bc->current = NULL;
+#endif
+
+    /* If there is no more batch contexts, notify the signal handler */
+    if (!bc)
+        CheckCtrlBreak(BREAK_OUTOFBATCH);
+}
+
+/*
+ * Exit all the nested batch calls.
+ */
+VOID ExitAllBatches(VOID)
+{
+    while (bc)
+        ExitBatch();
 }
 
 /*
@@ -343,7 +361,23 @@ INT Batch(LPTSTR fullname, LPTSTR firstword, LPTSTR param,
PARSED_COMMAND *Cmd)
     {
         Cmd = ParseCommand(NULL);
         if (!Cmd)
-            continue;
+        {
+            if (!bParseError)
+                continue;
+
+            /* Echo the pre-parsed batch file line on error */
+            if (bEcho && !bDisableBatchEcho)
+            {
+                if (!bIgnoreEcho)
+                    ConOutChar(_T('\n'));
+                PrintPrompt();
+                ConOutPuts(ParseLine);
+                ConOutChar(_T('\n'));
+            }
+            /* Stop all execution */
+            ExitAllBatches();
+            break;
+        }
 
         /* JPP 19980807 */
         /* Echo the command and execute it */
@@ -441,8 +475,7 @@ LPTSTR ReadBatchLine(VOID)
     /* User halt */
     if (CheckCtrlBreak(BREAK_BATCHFILE))
     {
-        while (bc)
-            ExitBatch();
+        ExitAllBatches();
         return NULL;
     }
 
diff --git a/base/shell/cmd/batch.h b/base/shell/cmd/batch.h
index 23a5dc425d5..de425ee2f11 100644
--- a/base/shell/cmd/batch.h
+++ b/base/shell/cmd/batch.h
@@ -45,8 +45,8 @@ extern TCHAR textline[BATCH_BUFFSIZE]; /* Buffer for reading Batch file
lines */
 
 
 LPTSTR FindArg(TCHAR, BOOL *);
-LPTSTR BatchParams(LPTSTR, LPTSTR);
 VOID   ExitBatch(VOID);
+VOID   ExitAllBatches(VOID);
 INT    Batch(LPTSTR, LPTSTR, LPTSTR, PARSED_COMMAND *);
 BOOL   BatchGetString(LPTSTR lpBuffer, INT nBufferLength);
 LPTSTR ReadBatchLine(VOID);
diff --git a/base/shell/cmd/cmd.c b/base/shell/cmd/cmd.c
index 36de37ed79d..97f346af852 100644
--- a/base/shell/cmd/cmd.c
+++ b/base/shell/cmd/cmd.c
@@ -772,10 +772,22 @@ INT
 ExecuteCommand(
     IN PARSED_COMMAND *Cmd)
 {
+#define SeenGoto() \
+    (bc && bc->current == NULL)
+
     PARSED_COMMAND *Sub;
     LPTSTR First, Rest;
     INT Ret = 0;
 
+    /*
+     * Do not execute any command if we are about to exit CMD, or about to
+     * change batch execution context, e.g. in case of a CALL / GOTO / EXIT.
+     */
+    if (!Cmd)
+        return 0;
+    if (bExit || SeenGoto())
+        return 0;
+
     if (!PerformRedirection(Cmd->Redirections))
         return 1;
 
@@ -799,14 +811,14 @@ ExecuteCommand(
     case C_QUIET:
     case C_BLOCK:
     case C_MULTI:
-        for (Sub = Cmd->Subcommands; Sub; Sub = Sub->Next)
+        for (Sub = Cmd->Subcommands; Sub && !SeenGoto(); Sub = Sub->Next)
             Ret = ExecuteCommand(Sub);
         break;
 
     case C_OR:
         Sub = Cmd->Subcommands;
         Ret = ExecuteCommand(Sub);
-        if (Ret != 0)
+        if ((Ret != 0) && !SeenGoto())
         {
             nErrorLevel = Ret;
             Ret = ExecuteCommand(Sub->Next);
@@ -816,7 +828,7 @@ ExecuteCommand(
     case C_AND:
         Sub = Cmd->Subcommands;
         Ret = ExecuteCommand(Sub);
-        if (Ret == 0)
+        if ((Ret == 0) && !SeenGoto())
             Ret = ExecuteCommand(Sub->Next);
         break;
 
@@ -835,6 +847,8 @@ ExecuteCommand(
 
     UndoRedirection(Cmd->Redirections, NULL);
     return Ret;
+
+#undef SeenGoto
 }
 
 INT
@@ -842,7 +856,7 @@ ExecuteCommandWithEcho(
     IN PARSED_COMMAND *Cmd)
 {
     /* Echo the reconstructed command line */
-    if (bEcho && !bDisableBatchEcho && Cmd->Type != C_QUIET)
+    if (bEcho && !bDisableBatchEcho && Cmd && (Cmd->Type !=
C_QUIET))
     {
         if (!bIgnoreEcho)
             ConOutChar(_T('\n'));
diff --git a/base/shell/cmd/cmd.h b/base/shell/cmd/cmd.h
index 972a09ff446..a6e780bdc6e 100644
--- a/base/shell/cmd/cmd.h
+++ b/base/shell/cmd/cmd.h
@@ -28,10 +28,10 @@
 #include "cmdver.h"
 #include "cmddbg.h"
 
-#define BREAK_BATCHFILE 1
-#define BREAK_OUTOFBATCH 2
-#define BREAK_INPUT 3
-#define BREAK_IGNORE 4
+#define BREAK_BATCHFILE     1
+#define BREAK_OUTOFBATCH    2 /* aka. BREAK_ENDOFBATCHFILES */
+#define BREAK_INPUT         3
+#define BREAK_IGNORE        4
 
 /* define some error messages */
 #define D_ON         _T("on")
@@ -338,6 +338,10 @@ VOID EchoCommand(PARSED_COMMAND *Cmd);
 TCHAR *Unparse(PARSED_COMMAND *Cmd, TCHAR *Out, TCHAR *OutEnd);
 VOID FreeCommand(PARSED_COMMAND *Cmd);
 
+void ParseErrorEx(LPTSTR s);
+extern BOOL bParseError;
+extern TCHAR ParseLine[CMDLINE_LENGTH];
+
 /* Prototypes from PATH.C */
 INT cmd_path (LPTSTR);
 
diff --git a/base/shell/cmd/for.c b/base/shell/cmd/for.c
index 08a5db3fa29..744d740b948 100644
--- a/base/shell/cmd/for.c
+++ b/base/shell/cmd/for.c
@@ -74,11 +74,12 @@ static BOOL GetNextElement(TCHAR **pStart, TCHAR **pEnd)
     ExecuteCommandWithEcho((Cmd)->Subcommands)
 
 /* Check if this FOR should be terminated early */
-static BOOL Exiting(PARSED_COMMAND *Cmd)
-{
-    /* Someone might have removed our context */
-    return bCtrlBreak || fc != Cmd->For.Context;
-}
+#define Exiting(Cmd) \
+    /* Someone might have removed our context */ \
+    (bCtrlBreak || (fc != (Cmd)->For.Context))
+/* Take also GOTO jumps into account */
+#define ExitingOrGoto(Cmd) \
+    (Exiting(Cmd) || (bc && bc->current == NULL))
 
 /* Read the contents of a text file into memory,
  * dynamically allocating enough space to hold it all */
@@ -251,7 +252,7 @@ static INT ForF(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR *Buffer)
 
     /* Loop over each file */
     End = List;
-    while (GetNextElement(&Start, &End))
+    while (!ExitingOrGoto(Cmd) && GetNextElement(&Start, &End))
     {
         FILE *InputFile;
         LPTSTR FullInput, In, NextLine;
@@ -301,9 +302,9 @@ static INT ForF(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR *Buffer)
         }
 
         /* Loop over the input line by line */
-        In = FullInput;
-        Skip = SkipLines;
-        do
+        for (In = FullInput, Skip = SkipLines;
+             !ExitingOrGoto(Cmd) && (In != NULL);
+             In = NextLine)
         {
             DWORD RemainingTokens = Tokens;
             LPTSTR *CurVar = Variables;
@@ -340,7 +341,8 @@ static INT ForF(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR *Buffer)
             /* Don't run unless the line had enough tokens to fill at least one
variable */
             if (*Variables[0])
                 Ret = RunInstance(Cmd);
-        } while (!Exiting(Cmd) && (In = NextLine) != NULL);
+        }
+
         cmd_free(FullInput);
     }
 
@@ -360,6 +362,11 @@ static INT ForLoop(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR *Buffer)
         params[i] = _tcstol(Start, NULL, 0);
 
     i = params[START];
+    /*
+     * Windows' CMD compatibility:
+     * Contrary to the other FOR-loops, FOR /L does not check
+     * whether a GOTO has been done, and will continue to loop.
+     */
     while (!Exiting(Cmd) &&
            (params[STEP] >= 0 ? (i <= params[END]) : (i >= params[END])))
     {
@@ -379,7 +386,7 @@ static INT ForDir(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR *Buffer,
TCHAR *BufPos
     INT Ret = 0;
     TCHAR *Start, *End = List;
 
-    while (!Exiting(Cmd) && GetNextElement(&Start, &End))
+    while (!ExitingOrGoto(Cmd) && GetNextElement(&Start, &End))
     {
         if (BufPos + (End - Start) > &Buffer[CMDLINE_LENGTH])
             continue;
@@ -410,7 +417,7 @@ static INT ForDir(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR *Buffer,
TCHAR *BufPos
                     continue;
                 _tcscpy(FilePart, w32fd.cFileName);
                 Ret = RunInstance(Cmd);
-            } while (!Exiting(Cmd) && FindNextFile(hFind, &w32fd));
+            } while (!ExitingOrGoto(Cmd) && FindNextFile(hFind, &w32fd));
             FindClose(hFind);
         }
         else
@@ -436,6 +443,12 @@ static INT ForRecursive(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR
*Buffer, TCHAR *
 
     Ret = ForDir(Cmd, List, Buffer, BufPos);
 
+    /* NOTE (We don't apply Windows' CMD compatibility here):
+     * Windows' CMD does not check whether a GOTO has been done,
+     * and will continue to loop. */
+    if (ExitingOrGoto(Cmd))
+        return Ret;
+
     _tcscpy(BufPos, _T("*"));
     hFind = FindFirstFile(Buffer, &w32fd);
     if (hFind == INVALID_HANDLE_VALUE)
@@ -448,7 +461,11 @@ static INT ForRecursive(PARSED_COMMAND *Cmd, LPTSTR List, TCHAR
*Buffer, TCHAR *
             _tcscmp(w32fd.cFileName, _T("..")) == 0)
             continue;
         Ret = ForRecursive(Cmd, List, Buffer, _stpcpy(BufPos, w32fd.cFileName));
-    } while (!Exiting(Cmd) && FindNextFile(hFind, &w32fd));
+
+    /* NOTE (We don't apply Windows' CMD compatibility here):
+     * Windows' CMD does not check whether a GOTO has been done,
+     * and will continue to loop. */
+    } while (!ExitingOrGoto(Cmd) && FindNextFile(hFind, &w32fd));
     FindClose(hFind);
 
     return Ret;
diff --git a/base/shell/cmd/goto.c b/base/shell/cmd/goto.c
index 2843e9618c1..10eeb28b12e 100644
--- a/base/shell/cmd/goto.c
+++ b/base/shell/cmd/goto.c
@@ -66,7 +66,11 @@ INT cmd_goto(LPTSTR param)
     /* jump to end of the file */
     if ( _tcsicmp( param, _T(":eof"))==0)
     {
-        bc->mempos=bc->memsize;		/* position at the end of the batchfile */
+        /* Position at the end of the batch file */
+        bc->mempos = bc->memsize;
+
+        /* Do not process any more parts of a compound command */
+        bc->current = NULL;
         return 0;
     }
 
@@ -102,7 +106,11 @@ INT cmd_goto(LPTSTR param)
         tmp2 = param;
         /* Use whole label name */
         if ((*tmp == _T(':')) && ((_tcsicmp(++tmp, param) == 0) ||
(_tcsicmp(tmp, ++tmp2) == 0)))
+        {
+            /* Do not process any more parts of a compound command */
+            bc->current = NULL;
             return 0;
+        }
     }
 
     ConErrResPrintf(STRING_GOTO_ERROR2, param);
diff --git a/base/shell/cmd/if.c b/base/shell/cmd/if.c
index 723bce32972..26f56dee95a 100644
--- a/base/shell/cmd/if.c
+++ b/base/shell/cmd/if.c
@@ -187,9 +187,7 @@ INT ExecuteIf(PARSED_COMMAND *Cmd)
     else
     {
         /* Full condition was false, do the "else" command if there is one */
-        if (Cmd->Subcommands->Next)
-            return ExecuteCommand(Cmd->Subcommands->Next);
-        return 0;
+        return ExecuteCommand(Cmd->Subcommands->Next);
     }
 }
 
diff --git a/base/shell/cmd/internal.c b/base/shell/cmd/internal.c
index d758c151a4c..9febc003cdc 100644
--- a/base/shell/cmd/internal.c
+++ b/base/shell/cmd/internal.c
@@ -531,9 +531,14 @@ INT CommandExit(LPTSTR param)
          * otherwise exit this command interpreter instance.
          */
         if (bc)
+        {
+            bc->current = NULL;
             ExitBatch();
+        }
         else
+        {
             bExit = TRUE;
+        }
     }
     else
     {
diff --git a/base/shell/cmd/parser.c b/base/shell/cmd/parser.c
index 88787efc7f5..e25b63aa91b 100644
--- a/base/shell/cmd/parser.c
+++ b/base/shell/cmd/parser.c
@@ -60,9 +60,9 @@ enum
 /* Scratch buffer for temporary command substitutions / expansions */
 static TCHAR TempBuf[CMDLINE_LENGTH];
 
-static BOOL bParseError;
+/*static*/ BOOL bParseError;
 static BOOL bLineContinuations;
-static TCHAR ParseLine[CMDLINE_LENGTH];
+/*static*/ TCHAR ParseLine[CMDLINE_LENGTH];
 static TCHAR *ParsePos;
 static TCHAR CurChar;
 
@@ -108,12 +108,19 @@ restart:
     return (CurChar = Char);
 }
 
-static void ParseError(void)
+void ParseErrorEx(LPTSTR s)
 {
-    error_syntax(CurrentTokenType != TOK_END ? CurrentToken : NULL);
+    /* Only display the first error we encounter */
+    if (!bParseError)
+        error_syntax(s);
     bParseError = TRUE;
 }
 
+static void ParseError(void)
+{
+    ParseErrorEx(CurrentTokenType != TOK_END ? CurrentToken : NULL);
+}
+
 /*
  * Yes, cmd has a Lexical Analyzer. Whenever the parser gives an "xxx was
  * unexpected at this time." message, it shows what the last token read was.