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.
-- v2: cmd: Avoid execution if block misses closing brackets. cmd: Handle lines with just spaces in bracket blocks.
From: Bernhard Übelacker bernhardu@mailbox.org
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51599 --- programs/cmd/tests/test_builtins.cmd | 26 ++++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 6 ++++++ programs/cmd/wcmdmain.c | 18 +++++++++++++++- 3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 510a1ba5931..44dfd080b90 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1029,6 +1029,32 @@ if not exist %windir% ( ) else ( echo windir does exist ) +if 1 == 0 ( + echo 1 == 0 should not be true +@space@ +) else echo block containing a line with just space seems to work +if 1 == 0 ( + echo 1 == 0 should not be true +@tab@ +) else echo block containing a line with just tab seems to work +if 1 == 0 ( + echo 1 == 0 should not be true +@space@@tab@ +) else echo block containing a line with just space and tab seems to work +if 1 == 0 ( + echo 1 == 0 should not be true +@tab@@space@ +) else echo block containing a line with just tab and space seems to work +if 1 == 0 ( + echo 1 == 0 should not be true +@space@ +@space@ +) else echo block containing two lines with just space seems to work +if 1 == 0 ( + echo 1 == 0 should not be true +@tab@ +@tab@ +) else echo block containing two lines with just tab 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 0f48a823109..fc1f9991df4 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -707,6 +707,12 @@ 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 space seems to work +block containing a line with just tab seems to work +block containing a line with just space and tab seems to work +block containing a line with just tab and space seems to work +block containing two lines with just space seems to work +block containing two lines with just tab 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 843fef8ea50..3759b1605da 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -1804,6 +1804,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 * @@ -2326,7 +2342,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 */
From: Bernhard Übelacker bernhardu@mailbox.org
--- programs/cmd/tests/test_builtins.cmd | 23 +++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 4 ++++ programs/cmd/wcmdmain.c | 7 +++++++ 3 files changed, 34 insertions(+)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 44dfd080b90..6aa8e956df3 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1055,6 +1055,29 @@ if 1 == 0 ( @tab@ @tab@ ) else echo block containing two lines with just tab seems to work +:: +echo @if 1 == 1 ^(> blockclosing.cmd +echo echo with closing bracket>> blockclosing.cmd +echo ^)>> blockclosing.cmd +cmd.exe /Q /C blockclosing.cmd +echo %ERRORLEVEL% ok +:: +echo @if 1 == 1 ^(> blockclosing.cmd +echo echo without closing bracket first>> blockclosing.cmd +echo echo without closing bracket second>> blockclosing.cmd +cmd.exe /Q /C blockclosing.cmd +echo %ERRORLEVEL% two lines not ok +:: +echo @if 1 == 1 ^(> blockclosing.cmd +echo echo before nested block without closing bracket>> blockclosing.cmd +echo @if 2 == 2 ^(>> blockclosing.cmd +echo echo without closing bracket>> blockclosing.cmd +echo ^)>> blockclosing.cmd +echo echo outside of block without closing bracket>> blockclosing.cmd +cmd.exe /Q /C blockclosing.cmd +echo %ERRORLEVEL% nested not ok +:: +del blockclosing.cmd 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 fc1f9991df4..435f4e9d2b8 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -713,6 +713,10 @@ block containing a line with just space and tab seems to work block containing a line with just tab and space seems to work block containing two lines with just space seems to work block containing two lines with just tab seems to work +with closing bracket +0 ok +255 two lines not ok +255 nested not ok --- 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 3759b1605da..ff9cd4b8db8 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -2362,6 +2362,13 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE } }
+ if (curDepth > lineCurDepth) { + WINE_TRACE("Brackets do not match, error out without executing.\n"); + WCMD_free_commands(*output); + *output = NULL; + errorlevel = 255; + } + /* Dump out the parsed output */ WCMD_DumpCommands(*output);
Sorry for the delay.
v2: - Rebased to current git - Changed the test, added e.g. two consecutive spaces only lines. - Added for now another patch to improve handling of unclosed blocks.
On Mon Jun 20 07:55:20 2022 +0000, eric pouech wrote:
Hello Bernhard, When WCMD_fgets() hits EOF, the break line 2309 exits 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.
Thanks for the clarification. I hope what the second patch does is really the issue you describe. But instead of trying to handle it inside the loop it tries to detect the unbalanced brackets afterwards, and removes the collected commands and sets errorlevel.
On Wed Feb 7 20:29:42 2024 +0000, Bernhard Übelacker wrote:
Sorry for the delay. v2:
- Rebased to current git
- Changed the test, added e.g. two consecutive spaces only lines.
- Added for now another patch to improve handling of unclosed blocks.
In the next iteration I would like to add one line to make sure cmd executes the line above an unclosed block. And see the carets for creating blockclosing.cmd are not needed.
And after looking through all shown related conversions I wonder if !4168 got closed maybe by accident?
eric pouech (@epo) commented about programs/cmd/wcmdmain.c:
} else break; }
} while (*extraData == 0x00);
} while (WCMD_isEmptyOrJustWhiteSpace(extraData));
just to avoid to introduce a new function, you could perhaps do something like this instead: ``` extradata = WCMD_skip_leading_space(extraData); } while (*extraData == 0x00); ``` (the rest of the patches look good to me)
-
Bernhard Übelacker -
Bernhard Übelacker (＠bernhardu)
-
eric pouech (＠epo)