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
-- v3: cmd: added support of FOR loop's rare var names (%%1), fixed expansion of outer var loop name inside inner loop
From: Dmitry Sokolov mr.dmitry.sokolov@gmail.com
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 --- programs/cmd/tests/test_builtins.cmd | 47 ++++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 9 +++++ 2 files changed, 56 insertions(+)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 510a1ba5931..2a617b2225b 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1604,6 +1604,53 @@ for %%i in (test) do ( ) echo d4 ) + +echo --- for loop with rare var names +set "WINE_LOG_LEVEL=" +:test_for_loop_params +set "WINE_ARGS= -sdkver=10.0.22000.0 -type=MD" +: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 ( + if "%WINE_LOG_LEVEL%" GEQ "2" ( + echo [DEBUG] inner argument {%%1, %%2} + ) + call :test_for_loop_params_inner %%1 %%2 + ) + set "WINE_ARGS=%%b" + goto :test_for_loop_params_parse +) +if not defined WINE_LOG_LEVEL set "WINE_LOG_LEVEL=1" & goto :test_for_loop_params +if "%WINE_LOG_LEVEL%" LSS "2" set "WINE_LOG_LEVEL=2" & goto :test_for_loop_params +goto :test_for_loop_params_end + +:test_for_loop_params_inner +set "__arg_found=" +if /I "%1"=="-sdkver" ( + echo SDKVER=%2 + set "__arg_found=1" +) +if /I "%1"=="-type" ( + echo TYPE=%2 + set "__arg_found=1" +) +if "%__arg_found%" NEQ "1" ( + if "%2"=="" ( + echo [ERROR] Invalid command line argument: '%1' + ) else ( + echo [ERROR] Invalid command line argument: '%1=%2' + ) + set "__arg_found=" + goto :eof +) +set "__arg_found=" +goto :eof + +:test_for_loop_params_end +set "WINE_ARGS=" +set "WINE_LOG_LEVEL=" + + 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..6a90f9d7584 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -1065,6 +1065,15 @@ d3 a4 c4 d4 +--- for loop with rare var names +SDKVER=10.0.22000.0 +TYPE=MD +SDKVER=10.0.22000.0 +TYPE=MD +[DEBUG] inner argument {-sdkver, 10.0.22000.0} +SDKVER=10.0.22000.0 +[DEBUG] inner argument {-type, MD} +TYPE=MD --- set /a ------ individual operations WINE_foo correctly 3
From: Dmitry Sokolov mr.dmitry.sokolov@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/builtins.c | 34 ++++++++++++++++++++++++++++++---- programs/cmd/wcmd.h | 18 ++++++++++++++---- programs/cmd/wcmdmain.c | 7 ++++--- 3 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index ca703af52ec..c8700a412a3 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] = { @@ -1914,6 +1915,28 @@ static int WCMD_for_nexttoken(int lasttoken, WCHAR *tokenstr, return nexttoken; }
+/************************************************************************** + * WCMD_check_varidx + * + * Checks if the given varidx and varidx+varoffset are in suitable range, + * i.e. (0 <= X <= 25) || (26 <= X <= 51) || (52 <= X <= 61) + * a z A Z 0 9 + * + * Parameters: + * varidx [I] - Var index to check + * varoffset [I] - Var offset to check + * + * Returns TRUE on success, FALSE otherwise + */ +inline BOOL WCMD_check_varidx(int varidx, int varoffset) { + int X = varidx + varoffset; + return (varoffset >= 0) && ( + (0 <= varidx && varidx <= 25 && 0 <= X && X <= 25) || + (26 <= varidx && varidx <= 51 && 26 <= X && X <= 51) || + (52 <= varidx && varidx <= 61 && 52 <= X && X <= 61) + ); +} + /************************************************************************** * WCMD_parse_line * @@ -1983,11 +2006,14 @@ static void WCMD_parse_line(CMD_LIST *cmdStart,
/* Empty out variables */ for (varoffset=0; - varidx >= 0 && varoffset<totalfound && (((varidx%26) + varoffset) < 26); + varidx >= 0 && varoffset<totalfound && WCMD_check_varidx(varidx, varoffset); 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 +2029,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 (!WCMD_check_varidx(varidx, varoffset)) break; }
/* Find the next token */ @@ -2014,7 +2040,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 && WCMD_check_varidx(varidx, varoffset)) { 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 +2241,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]); + 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/wcmd.h b/programs/cmd/wcmd.h index 1935bbcdcd2..53e3883683b 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -185,12 +185,22 @@ 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 +#define FOR_VAR_IDX(c) ( ((c)>='a'&&(c)<='z')\ + ?((c)-'a')\ + :( ((c)>='A'&&(c)<='Z')\ + ?(26+(c)-'A')\ + :( ((c)>='0'&&(c)<='9')\ + ?(52+(c)-'0')\ + :-1 ) ) ) +#define FOR_VAR_NAME(idx) ( (idx)>=52\ + ?('0'+((idx)-52))\ + :( (idx)>=26\ + ?('A'+((idx)-26))\ + :('a'+(idx)) ) )
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..8e7601b863d 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', + FOR_VAR_NAME(i), wine_dbgstr_w(forloopcontext.variable[i])); } } @@ -868,6 +868,7 @@ static void handleExpansion(WCHAR *cmd, BOOL atExecute, BOOL delayed) { int forvaridx = FOR_VAR_IDX(*(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);
From: Dmitry Sokolov mr.dmitry.sokolov@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 | 4 ++-- programs/cmd/tests/test_builtins.cmd | 10 ++++----- programs/cmd/tests/test_builtins.cmd.exp | 16 +++++++------- programs/cmd/wcmd.h | 28 ++++++++++++++---------- programs/cmd/wcmdmain.c | 4 ++-- 6 files changed, 35 insertions(+), 31 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 c8700a412a3..d8a74a6249f 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2002,7 +2002,7 @@ 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; @@ -2241,7 +2241,7 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) {
/* Variable should follow */ lstrcpyW(variable, thisArg); - 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 */ diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 2a617b2225b..f726b124bc1 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1608,7 +1608,7 @@ for %%i in (test) do ( echo --- for loop with rare var names set "WINE_LOG_LEVEL=" :test_for_loop_params -set "WINE_ARGS= -sdkver=10.0.22000.0 -type=MD" +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 ( @@ -1626,12 +1626,12 @@ goto :test_for_loop_params_end
:test_for_loop_params_inner set "__arg_found=" -if /I "%1"=="-sdkver" ( - echo SDKVER=%2 +if /I "%1"=="-boo" ( + echo foo=%2 set "__arg_found=1" ) -if /I "%1"=="-type" ( - echo TYPE=%2 +if /I "%1"=="-x" ( + echo x=%2 set "__arg_found=1" ) if "%__arg_found%" NEQ "1" ( diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 6a90f9d7584..e106a0d2f9a 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -1066,14 +1066,14 @@ a4 c4 d4 --- for loop with rare var names -SDKVER=10.0.22000.0 -TYPE=MD -SDKVER=10.0.22000.0 -TYPE=MD -[DEBUG] inner argument {-sdkver, 10.0.22000.0} -SDKVER=10.0.22000.0 -[DEBUG] inner argument {-type, MD} -TYPE=MD +foo=bar +x=y +foo=bar +x=y +[DEBUG] inner argument {-foo, bar} +foo=bar +[DEBUG] inner argument {-x, y} +x=y --- set /a ------ individual operations WINE_foo correctly 3 diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 53e3883683b..829c46100e5 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -186,18 +186,22 @@ 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 62 -#define FOR_VAR_IDX(c) ( ((c)>='a'&&(c)<='z')\ - ?((c)-'a')\ - :( ((c)>='A'&&(c)<='Z')\ - ?(26+(c)-'A')\ - :( ((c)>='0'&&(c)<='9')\ - ?(52+(c)-'0')\ - :-1 ) ) ) -#define FOR_VAR_NAME(idx) ( (idx)>=52\ - ?('0'+((idx)-52))\ - :( (idx)>=26\ - ?('A'+((idx)-26))\ - :('a'+(idx)) ) ) + +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'; +}
typedef struct _FOR_CONTEXT { WCHAR *variable[MAX_FOR_VARIABLES]; /* a-z then A-Z then 0-9 */ diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 8e7601b863d..f3f14a005c8 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -816,7 +816,7 @@ void handleExpansion(WCHAR *cmd, BOOL atExecute, BOOL delayed) { for (i=0;i<MAX_FOR_VARIABLES;i++) { if (forloopcontext.variable[i]) { WINE_TRACE("FOR variable context: %c = '%s'\n", - FOR_VAR_NAME(i), + get_FOR_var_name(i), wine_dbgstr_w(forloopcontext.variable[i])); } } @@ -865,7 +865,7 @@ 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]));
From: Dmitry Sokolov mr.dmitry.sokolov@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/tests/test_builtins.cmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index f726b124bc1..13d58acf567 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1626,7 +1626,7 @@ goto :test_for_loop_params_end
:test_for_loop_params_inner set "__arg_found=" -if /I "%1"=="-boo" ( +if /I "%1"=="-foo" ( echo foo=%2 set "__arg_found=1" )
On Tue Nov 28 18:31:46 2023 +0000, Dmitry Sokolov wrote:
Checking of the upper boundary is not enough. Var offset should correspond to the given var index.
right ; I meant (as varoffset is always >= 0) `varidx + varoffset < get_varidx_upper_boundary(varidx)`
by point is that I think that these loops could be better rewritten to show the underlying logic and moving that logic in a helper function called for each iteration makes it harder to read
note that varidx is constant in the function, so the upper bound could be computed once for all and used in all loops
(I wished that the error case varidx < 0 should had been dropped early instead of polluting all parts of the function)
On Tue Nov 28 18:27:28 2023 +0000, Dmitry Sokolov wrote:
It's done for testing purpose. It covers the specific test case.
my goal here is to have smaller testcase as possible (so we have the less to maintain afterwards)
could you explain:
* what would not be tested if the WINE_LOG_LOG_LEVEL loops where dropped? * what would not be tested if the :test_for_params_inner subfunction was reduced to "echo %%1 %%2"?
-
Dmitry Sokolov -
Dmitry Sokolov (＠DmitrySokolov)
-
eric pouech (＠epo)