[PATCH v5 0/1] MR4418: cmd: Delay variable substitution until after the if clause is parsed.

[Alex Henrie (＠alexhenrie)]

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

--
  v5: cmd: Delay variable substitution until after the if clause is parsed.

https://gitlab.winehq.org/wine/wine/-/merge_requests/4418

[PATCH v5 1/1] cmd: Delay variable substitution until after the if clause is parsed.

[Alex Henrie]

From: Alex Henrie

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55859
---
 programs/cmd/builtins.c                  | 35 +++++++++++++++++++-----
 programs/cmd/tests/test_builtins.cmd.exp |  2 +-
 programs/cmd/wcmd.h                      |  1 +
 programs/cmd/wcmdmain.c                  |  9 +++---
 4 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index bc9ba9e5f11..45db1dd44aa 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -2824,9 +2824,24 @@ static int evaluate_if_comparison(const WCHAR *leftOperand, const WCHAR *operato
     return -1;
 }

+static void do_delayed_expansion(WCHAR *p)
+{
+  /* Perform delayed substitution after the if clause is parsed */
+  if (!delayedsubst) return;
+  for (p = wcschr(p, '!'); p; p = wcschr(p, '!'))
+    WCMD_expand_envvar(p, '!');
+}
+
+static void expand_first_param(WCHAR *dst, WCHAR *src, int negate)
+{
+  wcscpy(dst, WCMD_parameter(src, 1 + negate, NULL, FALSE, FALSE));
+  do_delayed_expansion(dst);
+}
+
 int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
 {
   WCHAR condition[MAX_PATH];
+  WCHAR param[MAXSTRING];
   int caseInsensitive = (wcsstr(quals, L"/I") != NULL);

*negate = !lstrcmpiW(param1,L"not");
@@ -2834,9 +2849,10 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
   WINE_TRACE("Condition: %s\n", wine_dbgstr_w(condition));

if (!lstrcmpiW(condition, L"errorlevel")) {
-    WCHAR *param = WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE);
     WCHAR *endptr;
-    long int param_int = wcstol(param, &endptr, 10);
+    long int param_int;
+    expand_first_param(param, p, *negate);
+    param_int = wcstol(param, &endptr, 10);
     if (*endptr) goto syntax_err;
     *test = ((long int)errorlevel >= param_int);
     WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
@@ -2844,8 +2860,10 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
   else if (!lstrcmpiW(condition, L"exist")) {
     WIN32_FIND_DATAW fd;
     HANDLE hff;
-    WCHAR *param = WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE);
-    int    len = lstrlenW(param);
+    int len;
+
+    expand_first_param(param, p, *negate);
+    len = wcslen(param);

if (!len) {
         *test = FALSE;
@@ -2861,8 +2879,8 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
     WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
   }
   else if (!lstrcmpiW(condition, L"defined")) {
-    *test = (GetEnvironmentVariableW(WCMD_parameter(p, 1+(*negate), NULL, FALSE, FALSE),
-                                    NULL, 0) > 0);
+    expand_first_param(param, p, *negate);
+    *test = (GetEnvironmentVariableW(param, NULL, 0) > 0);
     WCMD_parameter(p, 2+(*negate), command, FALSE, FALSE);
   }
   else { /* comparison operation */
@@ -2889,12 +2907,15 @@ int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate)
     lstrcpyW(rightOperand, WCMD_parameter(p, 0, &paramStart, TRUE, FALSE));
     if (!*rightOperand)
       goto syntax_err;
+    p = paramStart + lstrlenW(rightOperand);
+
+    do_delayed_expansion(leftOperand);
+    do_delayed_expansion(rightOperand);

*test = evaluate_if_comparison(leftOperand, operator, rightOperand, caseInsensitive);
     if (*test == -1)
       goto syntax_err;

-    p = paramStart + lstrlenW(rightOperand);
     WCMD_parameter(p, 0, command, FALSE, FALSE);
   }

diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 76665765b26..b8a9061f7ef 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -633,7 +633,7 @@ bar@or_broken@foo
 0@or_broken@1
 foo
 !WINE_FOO!
-@todo_wine@not empty
+not empty
 --- using /V cmd flag
 foo
 foo@or_broken@!WINE_FOO!
diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h
index f538e4fc1a6..c972200077a 100644
--- a/programs/cmd/wcmd.h
+++ b/programs/cmd/wcmd.h
@@ -102,6 +102,7 @@ void WCMD_echo (const WCHAR *);
 void WCMD_endlocal (void);
 void WCMD_enter_paged_mode(const WCHAR *);
 void WCMD_exit (CMD_NODE **cmdList);
+WCHAR* WCMD_expand_envvar(WCHAR *start, WCHAR startchar);
 void WCMD_for (WCHAR *, CMD_NODE **cmdList);
 BOOL WCMD_get_fullpath(const WCHAR *, SIZE_T, WCHAR *, WCHAR **);
 void WCMD_give_help (const WCHAR *args);
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index 2c5cf4a3196..c251981dc92 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -551,7 +551,7 @@ static inline BOOL WCMD_is_magic_envvar(const WCHAR *s, const WCHAR *magicvar)
  *
  *	Expands environment variables, allowing for WCHARacter substitution
  */
-static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar)
+WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar)
 {
     WCHAR *endOfVar = NULL, *s;
     WCHAR *colonpos = NULL;
@@ -1383,9 +1383,10 @@ void WCMD_execute (const WCHAR *command, const WCHAR *redirects,
     }

/* Expand variables in command line mode only (batch mode will
-       be expanded as the line is read in, except for 'for' loops) */
-    handleExpansion(new_cmd, (context != NULL), delayedsubst);
-    handleExpansion(new_redir, (context != NULL), delayedsubst);
+       be expanded as the line is read in, except for 'for' loops).
+       Do delayed substitution now if it is enabled and the command is not IF. */
+    handleExpansion(new_cmd, context != NULL, delayedsubst && cmd_index != WCMD_IF);
+    handleExpansion(new_redir, context != NULL, delayedsubst && cmd_index != WCMD_IF);

/*
  * Changing default drive has to be handled as a special case, anything

[PATCH v5 1/1] cmd: Delay variable substitution until after the if clause is parsed.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=145860

Your paranoid android.

=== debian11b (64 bit WoW report) ===

winmm:
mci: Timeout

[PATCH v5 0/1] MR4418: cmd: Delay variable substitution until after the if clause is parsed. - closed

[Alex Henrie (＠alexhenrie)]

This merge request was closed by Alex Henrie.

[Alex Henrie (＠alexhenrie)]

This was fixed in a different way by https://gitlab.winehq.org/wine/wine/-/commit/6c39db0ca01a93003d2fbf2581fc2c3...