Regression introduced in f238e846e701d2039eceb51f2f6e9d936f8c791c. Therefore if conditions got influenced by values of the previous line.
Previous submission: https://www.winehq.org/pipermail/wine-devel/2020-March/162158.html
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47770 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48738 Signed-off-by: Bernhard Übelacker bernhardu@mailbox.org --- programs/cmd/builtins.c | 6 ++++++ programs/cmd/wcmdmain.c | 15 +++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 43c4d9efef3..8911a597eb9 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2938,9 +2938,15 @@ void WCMD_if (WCHAR *p, CMD_LIST **cmdList) int test; /* Condition evaluation result */ WCHAR *command;
+ /* Function evaluate_if_condition relies on the global variables quals, param1 and param2 + set in a call to WCMD_parse before */ if (evaluate_if_condition(p, &command, &test, &negate) == -1) goto syntax_err;
+ WINE_TRACE("p: %s, quals: %s, param1: %s, param2: %s, command: %s\n", + wine_dbgstr_w(p), wine_dbgstr_w(quals), wine_dbgstr_w(param1), + wine_dbgstr_w(param2), wine_dbgstr_w(command)); + /* Process rest of IF statement which is on the same line Note: This may process all or some of the cmdList (eg a GOTO) */ WCMD_part_execute(cmdList, command, TRUE, (test != negate)); diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 2834986bf9e..6efc1c677d2 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -1947,8 +1947,6 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE To be able to handle ('s in the condition part take as much as evaluate_if_condition would take and skip parsing it here. */ } else if (WCMD_keyword_ws_found(ifCmd, ARRAY_SIZE(ifCmd), curPos)) { - static const WCHAR parmI[] = {'/','I','\0'}; - static const WCHAR notW[] = {'n','o','t','\0'}; int negate; /* Negate condition */ int test; /* Condition evaluation result */ WCHAR *p, *command; @@ -1956,17 +1954,18 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE inIf = TRUE;
p = curPos+(ARRAY_SIZE(ifCmd)); - while (*p == ' ' || *p == '\t') { + while (*p == ' ' || *p == '\t') p++; - if (lstrcmpiW(WCMD_parameter(p, 0, NULL, TRUE, FALSE), notW) == 0) - p += lstrlenW(notW); - if (lstrcmpiW(WCMD_parameter(p, 0, NULL, TRUE, FALSE), parmI) == 0) - p += lstrlenW(parmI); - } + WCMD_parse (p, quals, param1, param2);
+ /* Function evaluate_if_condition relies on the global variables quals, param1 and param2 + set in a call to WCMD_parse before */ if (evaluate_if_condition(p, &command, &test, &negate) != -1) { int if_condition_len = command - curPos; + WINE_TRACE("p: %s, quals: %s, param1: %s, param2: %s, command: %s, if_condition_len: %d\n", + wine_dbgstr_w(p), wine_dbgstr_w(quals), wine_dbgstr_w(param1), + wine_dbgstr_w(param2), wine_dbgstr_w(command), if_condition_len); memcpy(&curCopyTo[*curLen], curPos, if_condition_len*sizeof(WCHAR)); (*curLen)+=if_condition_len; curPos+=if_condition_len;
Just to demonstrate the crash without the previous patch. Should it be included in the test suite?
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47770 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48738 Signed-off-by: Bernhard Übelacker bernhardu@mailbox.org --- programs/cmd/tests/test_builtins.cmd | 5 +++++ programs/cmd/tests/test_builtins.cmd.exp | 1 + 2 files changed, 6 insertions(+)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 4bf37a35bb8..118576ebac8 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1013,6 +1013,11 @@ if /i not (a)==(b) ( ) else ( echo comparison operators surrounded by brackets seem to be broken ) +echo --- checking for regression described in bug #48738, not output, just expected not to crash +if not defined windir echo not-defined +if not exist %windir% ( + rem +) 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 5bcd29b917c..3dd7c344443 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -699,6 +699,7 @@ comparison operators surrounded by brackets seem to work comparison operators surrounded by brackets seem to work comparison operators surrounded by brackets seem to work comparison operators surrounded by brackets seem to work +--- checking for regression described in bug #48738, not output, just expected not to crash --- case sensitivity with and without /i option if seems to default to case sensitivity if /i seems to work
Bernhard Übelacker bernhardu@mailbox.org writes:
Just to demonstrate the crash without the previous patch. Should it be included in the test suite?
Yes, but it should be a proper test, checking that we get the expected results for that specific sequence of calls.
Am 29.04.20 um 21:08 schrieb Alexandre Julliard:
Bernhard Übelacker bernhardu@mailbox.org writes:
Just to demonstrate the crash without the previous patch. Should it be included in the test suite?
Yes, but it should be a proper test, checking that we get the expected results for that specific sequence of calls.
Thanks for clarifying. Should then both go into a single patch?
-
Alexandre Julliard -
Bernhard Übelacker