[PATCH 0/1] MR277: cmd: Handle lines with just spaces in bracket blocks.

[Bernhard Übelacker (＠bernhardu)]

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

This patch attempts to ignore lines with just spaces inside bracket blocks like it does already for completely empty lines.

[PATCH 1/1] cmd: Handle lines with just spaces in bracket blocks.

[Bernhard Übelacker]

From: Bernhard Übelacker

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51599
Signed-off-by: Bernhard Übelacker 
---
 programs/cmd/tests/test_builtins.cmd     |  4 ++++
 programs/cmd/tests/test_builtins.cmd.exp |  1 +
 programs/cmd/wcmdmain.c                  | 18 +++++++++++++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 3f410e55166..56ef2a5f726 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -1020,6 +1020,10 @@ if not exist %windir% (
 ) else (
   echo windir does exist
 )
+if 1 == 0 (
+   echo 1 == 0 should not be true
+   @space@@tab@
+) else echo block containing a line with just spaces seems to work
 echo --- case sensitivity with and without /i option
 if bar==BAR echo if does not default to case sensitivity
 if not bar==BAR echo if seems to default to case sensitivity
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 8b6e0914112..7ce684ae6fa 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -701,6 +701,7 @@ comparison operators surrounded by brackets seem to work
 comparison operators surrounded by brackets seem to work
 windir is defined
 windir does exist
+block containing a line with just spaces seems to work
 --- 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 af54b209d83..1b2f07fdc47 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -1798,6 +1798,22 @@ static BOOL WCMD_IsEndQuote(const WCHAR *quote, int quoteIndex)
     return FALSE;
 }

+/***************************************************************************
+ * WCMD_isEmptyOrJustWhiteSpace
+ *
+ *   Returns TRUE if str is empty or contains just whitespace characters,
+ *   otherwise returns FALSE.
+ */
+static BOOL WCMD_isEmptyOrJustWhiteSpace(const WCHAR *str)
+{
+    while (*str != '\0') {
+        if (*str != ' ' && *str != '\t')
+            return FALSE;
+        str++;
+    }
+    return TRUE;
+}
+
 /***************************************************************************
  * WCMD_ReadAndParseLine
  *
@@ -2319,7 +2335,7 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE
             } else break;
           }

-        } while (*extraData == 0x00);
+        } while (WCMD_isEmptyOrJustWhiteSpace(extraData));
         curPos = extraSpace;

/* Skip preceding whitespace */

[eric pouech (＠epo)]

while you're at it:
- you could perhaps properly handle EOF condition from WCMD_fgets in the (enclosing) loop
- testing two consecutive "blank" lines could be interesting too
see https://www.winehq.org/pipermail/wine-devel/2022-February/206613.html
A+

[Bernhard Übelacker (＠bernhardu)]

Hello Eric, thanks for looking at it and sorry, I was not aware that there was already a patch submitted for this issue.

Could you please give some more details about this EOF handling? When WCMD_fgets returns NULL for EOF the loop is left by the break in line [2309](https://gitlab.winehq.org/wine/wine/-/blob/master/programs/cmd/wcmdmain.c#L2...

About the consecutive lines, those testbot runs [117281](https://testbot.winehq.org/JobDetails.pl?Key=117281) [117271](https://testbot.winehq.org/JobDetails.pl?Key=117271) have them, so I currently do not see an issue there, with both of our attempts to solve it?

[eric pouech (＠epo)]

On Mon Jun 20 07:36:40 2022 +0000, Bernhard Übelacker wrote:
> Hello Eric, thanks for looking at it and sorry, I was not aware that
> there was already a patch submitted for this issue.
> Could you please give some more details about this EOF handling? When
> WCMD_fgets returns NULL for EOF the loop is left by the break in line [2309](https://gitlab.winehq.org/wine/wine/-/blob/master/programs/cmd/wcmdmain.c#L2...
> About the consecutive lines, those testbot runs
> [117281](https://testbot.winehq.org/JobDetails.pl?Key=117281)
> [117271](https://testbot.winehq.org/JobDetails.pl?Key=117271) have them,
> so I currently do not see an issue there, with both of our attempts to
> solve it?

Hello Bernhard,

When WCMD_fgets() hits EOF, the break line 2309 exists from 'do' loop starting at 2305, but continues at line 2323 by using extraSpace (which should have been read through extraData line 2308, thus in case of EOF, will reuse previously successful line read from WCMD_fgets...). IMO the EOF info should we propagated somehow (perhaps by setting an empty string in output buffer before the break)

For the consecutive lines, I was suggesting extending the test with several lines made of blank data (instead of a single one), to extend the coverage of the tests
. Your first patch already handles it (as testbot shows). Sorry if I haven't been clear.