[PATCH v5 0/1] MR4504: cmd: fix for bug 55401
Visual Studio's native tool command prompt uses rare for loop variables: %%1, %%2. This test covers this case. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55401 -- v5: cmd: added support of FOR loop's rare var names (%%1), fixed expansion of outer var loop name inside inner loop https://gitlab.winehq.org/wine/wine/-/merge_requests/4504
From: Dmitry Sokolov <mr.dmitry.sokolov(a)gmail.com> Visual Studio's native tool command prompt uses rare FOR loop variables: %%1, %%2. This fix allows to use %%1 var in FOR loops. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55401 --- programs/cmd/batch.c | 4 ++-- programs/cmd/builtins.c | 16 ++++++++----- programs/cmd/tests/test_builtins.cmd | 14 +++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 3 +++ programs/cmd/wcmd.h | 30 ++++++++++++++++++++---- programs/cmd/wcmdmain.c | 9 +++---- 6 files changed, 60 insertions(+), 16 deletions(-) diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 3586cd45731..7ea51c3de6b 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -396,7 +396,7 @@ void WCMD_HandleTildeModifiers(WCHAR **start, BOOL atExecute) break; } else { - int foridx = FOR_VAR_IDX(*lastModifier); + int foridx = get_FOR_var_index(*lastModifier); /* Its a valid parameter identifier - OK */ if ((foridx >= 0) && (forloopcontext.variable[foridx] != NULL)) break; @@ -424,7 +424,7 @@ void WCMD_HandleTildeModifiers(WCHAR **start, BOOL atExecute) *lastModifier-'0' + context -> shift_count[*lastModifier-'0'], NULL, FALSE, TRUE)); } else { - int foridx = FOR_VAR_IDX(*lastModifier); + int foridx = get_FOR_var_index(*lastModifier); lstrcpyW(outputparam, forloopcontext.variable[foridx]); } diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index ca703af52ec..d2cb7745a3a 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -37,6 +37,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(cmd); extern int defaultColor; extern BOOL echo_mode; extern BOOL interactive; +extern void handleExpansion(WCHAR *cmd, BOOL atExecute, BOOL delayed); struct env_stack *pushd_directories; const WCHAR inbuilt[][10] = { @@ -1979,15 +1980,18 @@ static void WCMD_parse_line(CMD_LIST *cmdStart, lasttoken = -1; nexttoken = WCMD_for_nexttoken(lasttoken, forf_tokens, &totalfound, &starfound, &thisduplicate); - varidx = FOR_VAR_IDX(variable); + varidx = get_FOR_var_index(variable); /* Empty out variables */ for (varoffset=0; - varidx >= 0 && varoffset<totalfound && (((varidx%26) + varoffset) < 26); + varidx >= 0 && varoffset < totalfound && varidx+varoffset <= get_FOR_var_index_upper_boundary(varidx); varoffset++) { forloopcontext.variable[varidx + varoffset] = emptyW; } + /* Expand outer loop variables */ + if (wcschr(buffer, '%')) handleExpansion(buffer, (context != NULL), FALSE); + /* Loop extracting the tokens Note: nexttoken of 0 means there were no tokens requested, to handle the special case of tokens=* */ @@ -2003,7 +2007,7 @@ static void WCMD_parse_line(CMD_LIST *cmdStart, if (varidx >=0) { if (parm) forloopcontext.variable[varidx + varoffset] = xstrdupW(parm); varoffset++; - if (((varidx%26)+varoffset) >= 26) break; + if (varidx+varoffset > get_FOR_var_index_upper_boundary(varidx)) break; } /* Find the next token */ @@ -2014,7 +2018,7 @@ static void WCMD_parse_line(CMD_LIST *cmdStart, /* If all the rest of the tokens were requested, and there is still space in the variable range, write them now */ - if (!anyduplicates && starfound && varidx >= 0 && (((varidx%26) + varoffset) < 26)) { + if (!anyduplicates && starfound && varidx >= 0 && varidx+varoffset <= get_FOR_var_index_upper_boundary(varidx)) { nexttoken++; WCMD_parameter_with_delims(buffer, (nexttoken-1), &parm, FALSE, FALSE, forf_delims); WINE_TRACE("Parsed allremaining tokens (%d) as parameter %s\n", @@ -2215,8 +2219,8 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { /* Variable should follow */ lstrcpyW(variable, thisArg); - WINE_TRACE("Variable identified as %s\n", wine_dbgstr_w(variable)); - varidx = FOR_VAR_IDX(variable[1]); + varidx = get_FOR_var_index(variable[1]); + WINE_TRACE("Variable identified as %s (idx: %d)\n", wine_dbgstr_w(variable), varidx); /* Ensure line continues with IN */ thisArg = WCMD_parameter(p, parameterNo++, NULL, FALSE, FALSE); diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 510a1ba5931..6081b945a7e 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1604,6 +1604,20 @@ for %%i in (test) do ( ) echo d4 ) + +echo --- for loop with rare var names +set "WINE_ARGS= -foo=bar -x=y" +:test_for_loop_params_parse +for /F "tokens=1,* delims= " %%a in ("%WINE_ARGS%") do ( + for /F "tokens=1,2 delims==" %%1 in ("%%a") do ( + echo inner argument {%%1, %%2} + ) + set "WINE_ARGS=%%b" + goto :test_for_loop_params_parse +) +set "WINE_ARGS=" + + echo --- set /a goto :testseta diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0f48a823109..5a207686ff6 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -1065,6 +1065,9 @@ d3 a4 c4 d4 +--- for loop with rare var names +inner argument {-foo, bar} +inner argument {-x, y} --- set /a ------ individual operations WINE_foo correctly 3 diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 1935bbcdcd2..24a317878e0 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -185,12 +185,34 @@ typedef struct _DIRECTORY_STACK /* Data structure to for loop variables during for body execution, bearing in mind that for loops can be nested */ -#define MAX_FOR_VARIABLES 52 -#define FOR_VAR_IDX(c) (((c)>='a'&&(c)<='z')?((c)-'a'):\ - ((c)>='A'&&(c)<='Z')?(26+(c)-'A'):-1) +#define MAX_FOR_VARIABLES 62 + +static inline int get_FOR_var_index(WCHAR c) +{ + if (c >= 'a' && c <= 'z') { return c - 'a'; } + if (c >= 'A' && c <= 'Z') { return 26 + c - 'A'; } + if (c >= '0' && c <= '9') { return 52 + c - '0'; } + return -1; +} + +static inline WCHAR get_FOR_var_name(int index) +{ + if (index >= 52) { return '0' + index - 52; } + if (index >= 26) { return 'A' + index - 26; } + if (index >= 0) { return 'a' + index; } + return 'a'; +} + +static inline int get_FOR_var_index_upper_boundary(int index) +{ + if (index >= 52) { return 61; } + if (index >= 26) { return 51; } + if (index >= 0) { return 25; } + return 0; +} typedef struct _FOR_CONTEXT { - WCHAR *variable[MAX_FOR_VARIABLES]; /* a-z then A-Z */ + WCHAR *variable[MAX_FOR_VARIABLES]; /* a-z then A-Z then 0-9 */ } FOR_CONTEXT; /* diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 843fef8ea50..f3f14a005c8 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -794,7 +794,7 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar) * rather than at parse time, i.e. delayed expansion and for loops need to be * processed */ -static void handleExpansion(WCHAR *cmd, BOOL atExecute, BOOL delayed) { +void handleExpansion(WCHAR *cmd, BOOL atExecute, BOOL delayed) { /* For commands in a context (batch program): */ /* Expand environment variables in a batch file %{0-9} first */ @@ -813,10 +813,10 @@ static void handleExpansion(WCHAR *cmd, BOOL atExecute, BOOL delayed) { WCHAR *normalp; /* Display the FOR variables in effect */ - for (i=0;i<52;i++) { + for (i=0;i<MAX_FOR_VARIABLES;i++) { if (forloopcontext.variable[i]) { WINE_TRACE("FOR variable context: %c = '%s'\n", - i<26?i+'a':(i-26)+'A', + get_FOR_var_name(i), wine_dbgstr_w(forloopcontext.variable[i])); } } @@ -865,9 +865,10 @@ static void handleExpansion(WCHAR *cmd, BOOL atExecute, BOOL delayed) { WCMD_strsubstW(p, p+2, NULL, 0); } else { - int forvaridx = FOR_VAR_IDX(*(p+1)); + int forvaridx = get_FOR_var_index(*(p+1)); if (startchar == '%' && forvaridx != -1 && forloopcontext.variable[forvaridx]) { /* Replace the 2 characters, % and for variable character */ + WINE_TRACE("FOR variable substitute %%%c with '%s'\n", *(p+1), wine_dbgstr_w(forloopcontext.variable[forvaridx])); WCMD_strsubstW(p, p + 2, forloopcontext.variable[forvaridx], -1); } else if (!atExecute || startchar == '!') { p = WCMD_expand_envvar(p, startchar); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4504
thanks for the rebasing the part for adding %%1 support as FOR variable looks ok to me but the inner loop expansion doesn't: * it should be on a separate commit as it's a different subject (inner loop expansion is broken whatever the variables are used for nested loops) * it looks like the command (echo %%1 %%2) is expanded in the context of the first FOR loop, but not in the context of the second ; but expansion already happens for a simple loop, so I'm not sure the proposed change (adding a second call to handleExpansion) is correct, but I'll tend to think that the (first) expansion isn't done at the right time on the last point, there's another pending merge request (for delayed expansion in IF commands) which shows the same concerns (expansion not done when expected) so either you're up into looking for a better solution for the inner loop expansion, or if not I'll take over from here concerning the %%1 support (whatever fits you above), I suggest having it separated from the variable expansion (and in the test you don't need nested loop for that) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4504#note_54589
With the bug being closed as fixed, is this MR still needed? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4504#note_86476
no, this MR is no longer needed and can be closed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4504#note_86847
This merge request was closed by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4504
participants (5)
-
Alexandre Julliard (＠julliard) -
Dmitry Sokolov
-
Dmitry Sokolov (＠DmitrySokolov)
-
eric pouech (＠epo)
-
Fabian Maurer (＠DarkShadow44)