Fixes regression in f53d57c8549dc439eb73354bfd37acd1e4e86cfd
Due to an invalid error condition check, a FindFirstFile was not terminated with a FindClose, resulting in a held lock. This meant a sharing violation message was issued if the file/directory was then removed - This was noticed as a subdirectory was not being cleaned up in %temp% at the end of the tests - with the fix on it is correctly cleaned up.
Example recreate outside the existing tests
mkdir subdir echo something>subdir\bar if exist subdir\ba* echo y del subdir\bar rd subdir
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/cmd/builtins.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 57a41c4752..ff58484b9a 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2844,7 +2844,7 @@ void WCMD_if (WCHAR *p, CMD_LIST **cmdList) WIN32_FIND_DATAW fd; HANDLE hff = FindFirstFileW(WCMD_parameter(p, 1+negate, NULL, FALSE, FALSE), &fd); test = (hff != INVALID_HANDLE_VALUE ); - if (!test) FindClose(hff); + if (test) FindClose(hff);
WCMD_parameter(p, 2+negate, &command, FALSE, FALSE); }
Related to bug#38558
for /f allows a special syntax of tokens=* (rather than tokens=1* for example) which just means put the whole line into the next variable). Note the handling of the 'next variable' was wrong in the case of it being 'A' or 'a' as the wrap calculation was wrong, but this only affected using this new syntax.
The command referenced in this bug (which was raised about a problem in the Microsoft version of cmd, but I just thought I'd try it in wine's) works with this patch applied.
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/cmd/builtins.c | 28 +++++++++++++++--------- programs/cmd/tests/test_builtins.cmd | 12 ++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 5 +++++ 3 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index ff58484b9a..1ee4fcccf3 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1831,6 +1831,14 @@ static int WCMD_for_nexttoken(int lasttoken, WCHAR *tokenstr, int nextnumber1, nextnumber2 = -1; WCHAR *nextchar;
+ /* It is valid syntax tokens=* which just means get whole line */ + if (*pos == '*') { + if (doall) *doall = TRUE; + if (totalfound) (*totalfound)++; + nexttoken = 0; + break; + } + /* Get the next number */ nextnumber1 = strtoulW(pos, &nextchar, 10);
@@ -1959,22 +1967,22 @@ static void WCMD_parse_line(CMD_LIST *cmdStart, */ lasttoken = -1; nexttoken = WCMD_for_nexttoken(lasttoken, forf_tokens, &totalfound, - NULL, &thisduplicate); + &starfound, &thisduplicate); varidx = FOR_VAR_IDX(variable);
/* Empty out variables */ for (varoffset=0; - varidx >= 0 && varoffset<totalfound && ((varidx+varoffset)%26); + varidx >= 0 && varoffset<totalfound && (((varidx%26) + varoffset) < 26); varoffset++) { forloopcontext.variable[varidx + varoffset] = (WCHAR *)nullW; - /* Stop if we walk beyond z or Z */ - if (((varidx+varoffset) % 26) == 0) break; }
- /* Loop extracting the tokens */ + /* Loop extracting the tokens + Note: nexttoken of 0 means there were no tokens requested, to handle + the special case of tokens=* */ varoffset = 0; WINE_TRACE("Parsing buffer into tokens: '%s'\n", wine_dbgstr_w(buffer)); - while (varidx >= 0 && (nexttoken > lasttoken)) { + while (varidx >= 0 && (nexttoken > 0 && (nexttoken > lasttoken))) { anyduplicates |= thisduplicate;
/* Extract the token number requested and set into the next variable context */ @@ -1982,9 +1990,9 @@ static void WCMD_parse_line(CMD_LIST *cmdStart, WINE_TRACE("Parsed token %d(%d) as parameter %s\n", nexttoken, varidx + varoffset, wine_dbgstr_w(parm)); if (varidx >=0) { - forloopcontext.variable[varidx + varoffset] = heap_strdupW(parm); + if (parm) forloopcontext.variable[varidx + varoffset] = heap_strdupW(parm); varoffset++; - if (((varidx + varoffset) %26) == 0) break; + if (((varidx%26)+varoffset) >= 26) break; }
/* Find the next token */ @@ -1995,12 +2003,12 @@ 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+varoffset) % 26)) { + if (!anyduplicates && starfound && varidx >= 0 && (((varidx%26) + varoffset) < 26)) { nexttoken++; WCMD_parameter_with_delims(buffer, (nexttoken-1), &parm, FALSE, FALSE, forf_delims); WINE_TRACE("Parsed allremaining tokens (%d) as parameter %s\n", varidx + varoffset, wine_dbgstr_w(parm)); - forloopcontext.variable[varidx + varoffset] = heap_strdupW(parm); + if (parm) forloopcontext.variable[varidx + varoffset] = heap_strdupW(parm); }
/* Execute the body of the foor loop with these values */ diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 0a8122c5e3..5ca2427a8f 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1744,6 +1744,12 @@ for /f "tokens=1,2,3*" %%i in ("a b c d e f g") do echo h=%%h i=%%i j=%%j k=%%k for /f "tokens=1,1,3*" %%i in ("a b c d e f g") do echo h=%%h i=%%i j=%%j k=%%k l=%%l m=%%m n=%%n o=%%o for /f "tokens=2,2,3*" %%i in ("a b c d e f g") do echo h=%%h i=%%i j=%%j k=%%k l=%%l m=%%m n=%%n o=%%o for /f "tokens=3,2,3*" %%i in ("a b c d e f g") do echo h=%%h i=%%i j=%%j k=%%k l=%%l m=%%m n=%%n o=%%o +rem Special case tokens=* +echo 3.14>testfile +FOR /F "tokens=*" %%A IN (testfile) DO @echo 1:%%A,%%B +FOR /F "tokens=1*" %%A IN (testfile) DO @echo 2:%%A,%%B +FOR /F "tokens=2*" %%A IN (testfile) DO @echo 3:%%A,%%B +del testfile cd .. rd /s/q foobar echo ------ parameter splitting @@ -1756,6 +1762,12 @@ goto :forFParameterSplittingEnd echo %~0 %~1 %~2 %~3 %~4 %~5 goto :eof :forFParameterSplittingEnd +echo 3.14>testfile +FOR /F "delims=. tokens=*" %%A IN (testfile) DO @echo 4:%%A,%%B +FOR /F "delims=. tokens=1*" %%A IN (testfile) DO @echo 5:%%A,%%B +FOR /F "delims=. tokens=2*" %%A IN (testfile) DO @echo 6:%%A,%%B +FOR /F "delims=. tokens=3*" %%A IN (testfile) DO @echo 7:%%A,%%B +del testfile
echo ------------ Testing del ------------ echo abc > file diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 7f4f724c3b..0a7c75a2a2 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -1234,9 +1234,14 @@ h=%h i=a j=b k=c l=d e f g m=%m n=%n o=%o@or_broken@h=%h i=a j=b k=c l=d e f g m h=%h i=a j=c k= l= m=%m n=%n o=%o@or_broken@h=%h i=a j=c k= l= m= n=%n o=%o h=%h i=b j=c k= l= m=%m n=%n o=%o@or_broken@h=%h i=b j=c k= l= m= n=%n o=%o h=%h i=b j=c k= l= m=%m n=%n o=%o@or_broken@h=%h i=b j=c k= l= m= n=%n o=%o +1:3.14,%B +2:3.14, ------ parameter splitting :forFParameterSplittingFunc myparam1=myvalue1 myparam2=myparam2 mytest@space@@space@@space@ :forFParameterSplittingFunc myparam1=myvalue1 myparam2=myparam2 mytest@space@@space@@space@ +4:3.14,%B +5:3,14 +6:14, ------------ Testing del ------------ deleting 'file' errorlevel is 0, good
Fixes bug#40742
When parsing a command, after the first '/' we store the characters away in quals. The command itself can be MAXSTRING in bytes, but the quals was limited to MAX_PATH. This is incorrect, as you can provide very long qualifiers as well. Expand the space to allow the maximum size possible.
According to the bug, this can be triggered causing a trap - I tried hard to get a trap, unsuccessfully, but I was able to prove that the quals was being filled to a size far greater than the allocated space, verifying this problem was valid.
The patch itself was originally attached to bug 40742 by 'Brian' but was never submitted. I've remade the patch (not that you can tell, as it ends up identical) and verified it is valid. Not sure how to give credit to the original author in this case, but adding my sign-off.
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/cmd/wcmd.h | 2 +- programs/cmd/wcmdmain.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index d4d97a0067..8d6eb6b48e 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -204,7 +204,7 @@ typedef struct _FOR_CONTEXT { * (uppercased and concatenated) and parameters entered, with environment * variables and batch parameters substitution already done. */ -extern WCHAR quals[MAX_PATH], param1[MAXSTRING], param2[MAXSTRING]; +extern WCHAR quals[MAXSTRING], param1[MAXSTRING], param2[MAXSTRING]; extern DWORD errorlevel; extern BATCH_CONTEXT *context; extern FOR_CONTEXT forloopcontext; diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 0d02f1f388..8fe2d574e5 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -38,7 +38,7 @@ extern struct env_stack *pushd_directories;
BATCH_CONTEXT *context = NULL; DWORD errorlevel; -WCHAR quals[MAX_PATH], param1[MAXSTRING], param2[MAXSTRING]; +WCHAR quals[MAXSTRING], param1[MAXSTRING], param2[MAXSTRING]; BOOL interactive; FOR_CONTEXT forloopcontext; /* The 'for' loop context */ BOOL delayedsubst = FALSE; /* The current delayed substitution setting */
Fixes bug#44952
When inside a for loop, an 'if' statement is processed and the true part taken. Once all the commands in the true are processed, the else part is parsed, and a flag set to skip all commands in the else part. Unfortunately this flag is left on even when the if statement ends, meaning subsequent commands are also skipped.
Note: This fix does break one test, but it only worked by luck previously, so the test has been flagged similar to a number of others for the same issue as a todo for now. I'm working on sorting these out too alongside some other bugs.
Signed-off-by: Jason Edmeades us@edmeades.me.uk --- programs/cmd/builtins.c | 18 +++++++++++-- programs/cmd/tests/test_builtins.cmd | 34 ++++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 14 ++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 1ee4fcccf3..088632f214 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1571,10 +1571,12 @@ static void WCMD_part_execute(CMD_LIST **cmdList, const WCHAR *firstcmd, /* execute all appropriate commands */ curPosition = *cmdList;
- WINE_TRACE("Processing cmdList(%p) - delim(%d) bd(%d / %d)\n", + WINE_TRACE("Processing cmdList(%p) - delim(%d) bd(%d / %d) processThese(%d)\n", *cmdList, (*cmdList)->prevDelim, - (*cmdList)->bracketDepth, myDepth); + (*cmdList)->bracketDepth, + myDepth, + processThese);
/* Execute any statements appended to the line */ /* FIXME: Only if previous call worked for && or failed for || */ @@ -1613,6 +1615,18 @@ static void WCMD_part_execute(CMD_LIST **cmdList, const WCHAR *firstcmd, if (*cmd) { WCMD_execute (cmd, (*cmdList)->redirects, cmdList, FALSE); } + } else { + /* Loop skipping all commands until we get back to the current + depth, including skipping commands and their subsequent + pipes (eg cmd | prog) */ + do { + *cmdList = (*cmdList)->nextcommand; + } while (*cmdList && + ((*cmdList)->bracketDepth > myDepth || + (*cmdList)->prevDelim)); + + /* After the else is complete, we need to now process subsequent commands */ + processThese = TRUE; } if (curPosition == *cmdList) *cmdList = (*cmdList)->nextcommand; } else if (!processThese) { diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 5ca2427a8f..b9e9b259a9 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -418,6 +418,7 @@ if 1==1 (echo n1) else echo n2|echo n3 if 1==1 (echo o1) else echo o2&&echo o3 if 1==1 (echo p1) else echo p2||echo p3 if 1==1 (echo q1) else echo q2&echo q3 +echo --- echo --- chain else (if false) if 1==0 echo a1 else echo a2 if 1==0 echo b1|echo b2 else echo b3 @@ -1371,6 +1372,39 @@ for /L %%i in (2,2,1) do ( echo %%i echo FAILED ) +echo --- ifs inside for loops +for %%i in (test) do ( + echo a1 + if 1==1 ( + echo b1 + ) else ( + echo c1 + ) + echo d1 +) +for %%i in (test) do ( + echo a2 + if 1==1 ( + echo b2 + ) else echo c2 + echo d2 +) +for %%i in (test) do ( + echo a3 + if 1==0 ( + echo b3 + ) else echo c3 + echo d3 +) +for %%i in (test) do ( + echo a4 + if 1==0 ( + echo b4 + ) else ( + echo c4 + ) + echo d4 +) echo --- set /a goto :testseta
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0a7c75a2a2..3118359265 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -428,6 +428,7 @@ n1 o1 p1 q1 +@todo_wine@--- --- chain else (if false) @todo_wine@j3 --- @@ -982,6 +983,19 @@ ErrorLevel 0 -1 1 3 +--- ifs inside for loops +a1 +b1 +d1 +a2 +b2 +d2 +a3 +c3 +d3 +a4 +c4 +d4 --- set /a ------ individual operations WINE_foo correctly 3
-
Jason Edmeades