Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58027
Also fix the find command, so it works in the new tests. This commit is the same as in !8589, it's needed by both.
From: Alfred Agrell floating@muncher.se
Without that, find ignores all arguments, and instantly returns errorlevel 1 and no output.
Other builtins and programs may also react to invalid stdin; I didn't investigate it. --- programs/cmd/tests/batch.c | 10 +++++++++- programs/cmd/tests/test_builtins.bat.exp | 16 ++++++++-------- programs/cmd/tests/test_builtins.cmd.exp | 16 ++++++++-------- 3 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/programs/cmd/tests/batch.c b/programs/cmd/tests/batch.c index 03a05f34215..79da6b011a0 100644 --- a/programs/cmd/tests/batch.c +++ b/programs/cmd/tests/batch.c @@ -112,7 +112,7 @@ static BOOL run_cmd(const char *res_name, const char *cmd_data, DWORD cmd_size) char *command; STARTUPINFOA si = {sizeof(si)}; PROCESS_INFORMATION pi; - HANDLE file,fileerr; + HANDLE file,fileerr,filenul; DWORD size; BOOL bres;
@@ -143,9 +143,16 @@ static BOOL run_cmd(const char *res_name, const char *cmd_data, DWORD cmd_size) if(fileerr == INVALID_HANDLE_VALUE) return FALSE;
+ filenul = CreateFileA("NUL", GENERIC_READ, FILE_SHARE_WRITE|FILE_SHARE_READ, &sa, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + ok(filenul != INVALID_HANDLE_VALUE, "CreateFile stdin failed\n"); + if(filenul == INVALID_HANDLE_VALUE) + return FALSE; + si.dwFlags = STARTF_USESTDHANDLES; si.hStdOutput = file; si.hStdError = fileerr; + si.hStdInput = filenul; bres = CreateProcessA(NULL, command, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); ok(bres, "CreateProcess failed: %lu\n", GetLastError()); if(!bres) { @@ -158,6 +165,7 @@ static BOOL run_cmd(const char *res_name, const char *cmd_data, DWORD cmd_size) CloseHandle(pi.hProcess); CloseHandle(file); CloseHandle(fileerr); + CloseHandle(filenul); DeleteFileA(command); return TRUE; } diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index a6b583f78c1..36e6b527a17 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -74,7 +74,7 @@ FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 -FAILURE 1 +@todo_wine@SUCCESS 0 FAILURE 1 FAILURE 1 --- success/failure for MOVE command @@ -174,7 +174,7 @@ SUCCESS 0 FAILURE 1 FAILURE 1 --- success/failure for LABEL command -FAILURE 1 +@todo_wine@SUCCESS 0 --- success/failure for PATH command SUCCESS 666 SUCCESS 666 @@ -218,16 +218,16 @@ FAILURE 1 SUCCESS 666 SUCCESS 666 --- success/failure for CHOICE command -FAILURE 1 -FAILURE 1 +@todo_wine@FAILURE 255 +@todo_wine@FAILURE 255 FAILURE 2 -FAILURE 1 +@todo_wine@FAILURE 255 --- success/failure for MORE command -SUCCESS 0 -SUCCESS 0 +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 foo@space@
SUCCESS 0 --- success/failure for PAUSE command -FAILURE 1 +@todo_wine@SUCCESS 666 --- diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 1d31b19e01c..de28e463e14 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -535,7 +535,7 @@ FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 -FAILURE 1 +@todo_wine@SUCCESS 0 FAILURE 1 FAILURE 1 --- success/failure for MOVE command @@ -635,7 +635,7 @@ SUCCESS 0 FAILURE 1 FAILURE 1 --- success/failure for LABEL command -FAILURE 1 +@todo_wine@SUCCESS 0 --- success/failure for PATH command SUCCESS 0 SUCCESS 0 @@ -679,18 +679,18 @@ FAILURE 1 SUCCESS 666 SUCCESS 666 --- success/failure for CHOICE command -FAILURE 1 -FAILURE 1 +@todo_wine@FAILURE 255 +@todo_wine@FAILURE 255 FAILURE 2 -FAILURE 1 +@todo_wine@FAILURE 255 --- success/failure for MORE command -SUCCESS 0 -SUCCESS 0 +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 foo@space@
SUCCESS 0 --- success/failure for PAUSE command -FAILURE 1 +@todo_wine@SUCCESS 666 --- ------------ Testing 'set' ------------ 1
From: Alfred Agrell floating@muncher.se
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58027 --- programs/cmd/builtins.c | 69 +++++++--- programs/cmd/tests/test_builtins.cmd | 153 +++++++++++++++++++++-- programs/cmd/tests/test_builtins.cmd.exp | 58 ++++++++- 3 files changed, 252 insertions(+), 28 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 21f15c32c4a..6b97e659b4f 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1338,10 +1338,10 @@ static BOOL WCMD_delete_confirm_wildcard(const WCHAR *filename, BOOL *pPrompted) * If /S was given, does it recursively. * Returns TRUE if a file was deleted. */ -static BOOL WCMD_delete_one (const WCHAR *thisArg) { +static BOOL WCMD_delete_one (const WCHAR *thisArg, WCHAR *notFoundFile) { DWORD wanted_attrs; DWORD unwanted_attrs; - BOOL found = FALSE; + BOOL deletedAny = FALSE; WCHAR argCopy[MAX_PATH]; WIN32_FIND_DATAW fd; HANDLE hff; @@ -1355,7 +1355,7 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) { WINE_TRACE("del: Processing arg %s (quals:%s)\n", wine_dbgstr_w(argCopy), wine_dbgstr_w(quals));
- if (!WCMD_delete_confirm_wildcard(argCopy, &found)) { + if (!WCMD_delete_confirm_wildcard(argCopy, &deletedAny)) { /* Skip this arg if user declines to delete *.* */ return FALSE; } @@ -1364,8 +1364,9 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) { hff = FindFirstFileW(argCopy, &fd); if (hff == INVALID_HANDLE_VALUE) { handleParm = FALSE; - } else { - found = TRUE; + if (!*notFoundFile) { + lstrcpyW(notFoundFile, thisArg); + } }
/* Support del <dirname> by just deleting all files dirname* */ @@ -1379,8 +1380,7 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) { lstrcpyW(modifiedParm, argCopy); lstrcatW(modifiedParm, L"\*"); FindClose(hff); - found = TRUE; - WCMD_delete_one(modifiedParm); + WCMD_delete_one(modifiedParm, notFoundFile);
} else if (handleParm) {
@@ -1421,7 +1421,11 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) { }
/* Now do the delete */ - if (!DeleteFileW(fpath)) WCMD_print_error (); + if (DeleteFileW(fpath)) { + deletedAny = TRUE; + } else { + errorlevel = GetLastError(); + } }
} @@ -1497,13 +1501,12 @@ static BOOL WCMD_delete_one (const WCHAR *thisArg) {
/* Go through each subdir doing the delete */ while (allDirs != NULL) { - found |= WCMD_delete_one (allDirs->dirName); + deletedAny |= WCMD_delete_one (allDirs->dirName, notFoundFile); allDirs = WCMD_dir_stack_free(allDirs); } } } - - return found; + return deletedAny; }
/**************************************************************************** @@ -1524,8 +1527,34 @@ RETURN_CODE WCMD_delete(WCHAR *args) int argno; WCHAR *argN; BOOL argsProcessed = FALSE; + BOOL deletedAny = FALSE; + WCHAR notFoundFile[MAX_PATH];
errorlevel = NO_ERROR; + *notFoundFile = L'\0'; + + for (argno = 0; ; argno++) + { + WCHAR *thisArg, *slash; + + argN = NULL; + thisArg = WCMD_parameter(args, argno, &argN, FALSE, FALSE); + if (!argN) + break; /* no more parameters */ + if (argN[0] == '/') + continue; /* skip options */ + if (!(slash = wcschr(argN, '\'))) + continue; + + *slash = L'\0'; + if (GetFileAttributesW(thisArg) == INVALID_FILE_ATTRIBUTES) { + SetLastError(ERROR_FILE_NOT_FOUND); + WCMD_print_error (); + errorlevel = 1; + return errorlevel; + } + *slash = L'\'; + }
for (argno = 0; ; argno++) { @@ -1539,17 +1568,25 @@ RETURN_CODE WCMD_delete(WCHAR *args) continue; /* skip options */
argsProcessed = TRUE; - if (!WCMD_delete_one(thisArg)) - { - errorlevel = ERROR_INVALID_FUNCTION; - } + deletedAny |= WCMD_delete_one(thisArg, notFoundFile); + if (errorlevel != NO_ERROR) + break; }
/* Handle no valid args */ if (!argsProcessed) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOARG)); - errorlevel = ERROR_INVALID_FUNCTION; + errorlevel = 1; + } + if (*notFoundFile && !deletedAny) + { + WCMD_output_stderr(WCMD_LoadMessage(WCMD_FILENOTFOUND), notFoundFile); + } + if (errorlevel) + { + SetLastError(errorlevel); + WCMD_print_error (); }
return errorlevel; diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 7db955f634d..f4469bb5118 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -2597,25 +2597,158 @@ for /f "tokens=*" %%A in (nul_test_file) DO echo %%A echo ------------ Testing del ------------ echo abc > file echo deleting 'file' -del file -if errorlevel 0 ( - echo errorlevel is 0, good -) else ( - echo unexpected errorlevel, got %errorlevel% -) +del file 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% if not exist file ( echo successfully deleted 'file' ) else ( echo error deleting 'file' ) echo attempting to delete 'file', even though it is not present -del file -if errorlevel 0 ( - echo errorlevel is 0, good +del file 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% + +echo deleting nul will fail +del nul 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% + +echo failing to delete nul cancels subsequent processing +echo abc > exists +del nul exists 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +if exist exists ( + echo didn't delete 'exists' + del exists ) else ( - echo unexpected errorlevel, got %errorlevel% + echo unexpectedly deleted 'exists' )
+echo but if the existing file is first, it is of course deleted +echo abc > exists +del exists nul 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +if not exist file ( + echo successfully deleted 'exists' +) else ( + echo error deleting 'exists' +) + +echo deleting the same file twice does not complain +echo abc > exists +del exists exists 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +if not exist exists ( + echo successfully deleted 'exists' +) else ( + echo error deleting 'exists' +) + +echo deleting two files that don't exist complains about the first one +del exists1 exists2 2>del.log +echo %errorlevel% +find "exists1" del.log >nul +echo search exists1 %errorlevel% +find "exists2" del.log >nul +echo search exists2 %errorlevel% + +echo deleting a file that doesn't exist, then one that does, deletes it without complaining +echo abc > exists +del doesntexist exists 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +if not exist exists ( + echo successfully deleted 'exists' +) else ( + echo error deleting 'exists' +) + +echo deleting a file that doesn't exist, then one that can't be deleted, complains about both +del doesntexist nul 2>del.log +echo %errorlevel% +find "doesntexist" del.log >nul +echo search doesntexist %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% + +echo deleting a file that doesn't exist, then one that does, then one that can't be deleted, complains about latter only +echo abc > exists +del doesntexist exists nul 2>del.log +echo %errorlevel% +find "doesntexist" del.log >nul +echo search doesntexist %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +if not exist exists ( + echo successfully deleted 'exists' +) else ( + echo error deleting 'exists' +) + +echo however, if the filename's directory doesn't exist, it's a hard error, just like undeletable files +del notadirectory\file.txt 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +find "file.txt" del.log >nul +echo search file.txt %errorlevel% + +echo like nul, it stops argument processing +echo abc > exists +del notadirectory\file.txt exists 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +if exist exists ( + echo didn't delete 'exists' + del exists +) else ( + echo unexpectedly deleted 'exists' +) + +echo unlike deleting nonexistent+nul, it only complains about one +del doesntexist notadirectory\file.txt 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +find "doesntexist" del.log >nul +echo search doesntexist %errorlevel% +find "file.txt" del.log >nul +echo search file.txt %errorlevel% + +echo and unlike nul, it stops processing before anything is deleted +echo abc > exists +del exists nul notadirectory\file.txt nul 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% +if exist exists ( + echo didn't delete 'exists' + del exists +) else ( + echo unexpectedly deleted 'exists' +) + +echo and with no arguments, it prints an error +del 2>del.log +echo %errorlevel% +findstr . del.log >nul +echo empty %errorlevel% + +del del.log + echo ------------ Testing del /a ------------ del /f/q *.test > nul echo r > r.test diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index de28e463e14..f18ef369f2b 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -1664,10 +1664,64 @@ a a b c ------------ Testing del ------------ deleting 'file' -errorlevel is 0, good +0 +empty 1 successfully deleted 'file' attempting to delete 'file', even though it is not present -errorlevel is 0, good +0 +empty 0 +deleting nul will fail +@todo_wine@123 +empty 0 +failing to delete nul cancels subsequent processing +@todo_wine@123 +empty 0 +didn't delete 'exists' +but if the existing file is first, it is of course deleted +@todo_wine@123 +empty 0 +successfully deleted 'exists' +deleting the same file twice does not complain +0 +empty 1 +successfully deleted 'exists' +deleting two files that don't exist complains about the first one +0 +search exists1 0 +search exists2 1 +deleting a file that doesn't exist, then one that does, deletes it without complaining +0 +empty 1 +successfully deleted 'exists' +deleting a file that doesn't exist, then one that can't be deleted, complains about both +@todo_wine@123 +search doesntexist 0 +empty 0 +deleting a file that doesn't exist, then one that does, then one that can't be deleted, complains about latter only +@todo_wine@123 +search doesntexist 1 +empty 0 +successfully deleted 'exists' +however, if the filename's directory doesn't exist, it's a hard error, just like undeletable files +1 +empty 0 +search file.txt 1 +like nul, it stops argument processing +1 +empty 0 +didn't delete 'exists' +unlike deleting nonexistent+nul, it only complains about one +1 +empty 0 +search doesntexist 1 +search file.txt 1 +and unlike nul, it stops processing before anything is deleted +1 +empty 0 +didn't delete 'exists' +and with no arguments, it prints an error +1 +empty 0 ------------ Testing del /a ------------ not-r.test not found after delete, good r.test found before delete, good
1) each individual test can decide upon its input stream (<NUL, <CON, echo "xyz" | ...); so there's no absolute need to bind cmd's std input to NUL 2) if cmd's input has to be changed, IMO it should be bound to console's input (as some builtin commands use stdin, other reopen the console; so that's a saner default). Didn't test it, but passing GetStdHandle(STD_INPUT_HANDLE) in CreateProcess instead could help.
On Fri Jul 18 09:04:29 2025 +0000, eric pouech wrote:
- each individual test can decide upon its input stream (<NUL, <CON,
echo "xyz" | ...); so there's no absolute need to bind cmd's std input to NUL 2) if cmd's input has to be changed, IMO it should be bound to console's input (as some builtin commands use stdin, other reopen the console; so that's a saner default). Didn't test it, but passing GetStdHandle(STD_INPUT_HANDLE) in CreateProcess instead could help. that will also force to adapt all the tests that read from stdin to explicitly define what input they're using
Technically true, but adding <nul to every find invocation whose args say don't read stdin would be ugly, confusing to future readers, and easy to forget in future find calls. I'd rather fix the root cause.
No opinion on nul vs caller's stdin; if you feel stdin is better, sure, let's do that. Needs a few additional <nul so date, time and choice don't get stuck waiting for user input, but that's probably an improvement anyways.
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
eric pouech (＠epo)