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.
-- v2: cmd: Fix errorlevel from del. cmd/tests: Pass a valid handle as hStdInput.
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 | 1 + programs/cmd/tests/test_builtins.bat | 6 +++--- programs/cmd/tests/test_builtins.bat.exp | 16 ++++++++-------- programs/cmd/tests/test_builtins.cmd | 6 +++--- programs/cmd/tests/test_builtins.cmd.exp | 16 ++++++++-------- 5 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/programs/cmd/tests/batch.c b/programs/cmd/tests/batch.c index 03a05f34215..18af26f9677 100644 --- a/programs/cmd/tests/batch.c +++ b/programs/cmd/tests/batch.c @@ -146,6 +146,7 @@ static BOOL run_cmd(const char *res_name, const char *cmd_data, DWORD cmd_size) si.dwFlags = STARTF_USESTDHANDLES; si.hStdOutput = file; si.hStdError = fileerr; + si.hStdInput = GetStdHandle(STD_INPUT_HANDLE); bres = CreateProcessA(NULL, command, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); ok(bres, "CreateProcess failed: %lu\n", GetLastError()); if(!bres) { diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index 728a0e4eafc..7bff7c92c44 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -91,7 +91,7 @@ call :setError 666 & (start "" /foobar >NUL &&echo SUCCESS !errorlevel!||echo FA rem call :setError 666 & (start /B I\dont\exist.exe &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) rem can't run this test, generates a nice popup under windows call :setError 666 & (start "" /B /WAIT cmd.exe /c "echo foo & exit /b 1024" &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (start "" /B cmd.exe /c "(choice /C:YN /T:3 /D:Y > NUL) & exit /b 1024" &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (start "" /B cmd.exe /c "(choice /C:YN /T:3 /D:Y > NUL < NUL) & exit /b 1024" &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- success/failure for TYPE command mkdir foo & cd foo echo a > fileA @@ -243,11 +243,11 @@ call :setError 666 & (setlocal DisableExtensions &&echo SUCCESS !errorlevel!||ec call :setError 666 & (setlocal EnableExtensions &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- success/failure for DATE command call :setError 666 & (date /t >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (date AAAA >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (date AAAA <NUL >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) rem need evelated priviledges to set the date echo --- success/failure for TIME command call :setError 666 & (time /t >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (time AAAA >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (time AAAA <NUL >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) rem need evelated priviledges to set the time echo --- success/failure for BREAK command call :setError 666 & (break &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) 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 b/programs/cmd/tests/test_builtins.cmd index 7db955f634d..c8fcaa3f54e 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -608,7 +608,7 @@ call :setError 666 & (start "" /foobar >NUL &&echo SUCCESS !errorlevel!||echo FA rem call :setError 666 & (start /B I\dont\exist.exe &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) rem can't run this test, generates a nice popup under windows call :setError 666 & (start "" /B /WAIT cmd.exe /c "echo foo & exit /b 1024" &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (start "" /B cmd.exe /c "(choice /C:YN /T:3 /D:Y > NUL) & exit /b 1024" &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (start "" /B cmd.exe /c "(choice /C:YN /T:3 /D:Y > NUL < NUL) & exit /b 1024" &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- success/failure for TYPE command mkdir foo & cd foo echo a > fileA @@ -760,11 +760,11 @@ call :setError 666 & (setlocal DisableExtensions &&echo SUCCESS !errorlevel!||ec call :setError 666 & (setlocal EnableExtensions &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- success/failure for DATE command call :setError 666 & (date /t >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (date AAAA >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (date AAAA <NUL >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) rem need evelated priviledges to set the date echo --- success/failure for TIME command call :setError 666 & (time /t >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (time AAAA >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (time AAAA <NUL >NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) rem need evelated priviledges to set the time echo --- success/failure for BREAK command call :setError 666 & (break &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) 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 c8fcaa3f54e..7b20522ef71 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
On Fri Jul 18 09:04:29 2025 +0000, Alfred Agrell wrote:
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.
I'm still thinking of the best approach... - on one hand, using an invalid handle as default for stdin, even if a valid situation, is not the most common - on the other hand, using NUL input, or CONsole's input, or a pipe:d string is doable from tests; I don't see a simple way of using an invalid handle from the tests so if we want to be able to cover all possible test cases, keeping current invalid handle as default is perhaps the best solution
if we now consider what we're testing, there are a couple of tests which don't take care of explicitely defining their input (while they should, as behavior depends on input) from that, there are likely a bunch of them for which return_code/errorlevel will only differ because of the read(stdin) result (error for invalid handle vs empty buffer for NUL). of course, there are a few others (CHOICE coming to mind first) where defining input is a must have. Passing input handle in CreateProcess will block input, hence identify the potential culprits. But this will be another pain on test writer to understand why the test times out.
Perhaps, having a first shot at which tests trigger reading from input could help in deciding on best approach.
how to you get the issues with find? AFAICS the cmd.exe tests only use the form with a path specified, which shouldn't read from stdin. so maybe there's something else lurking here
On Fri Jul 18 10:08:35 2025 +0000, eric pouech wrote:
I'm still thinking of the best approach...
- on one hand, using an invalid handle as default for stdin, even if a
valid situation, is not the most common
- on the other hand, using NUL input, or CONsole's input, or a pipe:d
string is doable from tests; I don't see a simple way of using an invalid handle from the tests so if we want to be able to cover all possible test cases, keeping current invalid handle as default is perhaps the best solution if we now consider what we're testing, there are a couple of tests which don't take care of explicitely defining their input (while they should, as behavior depends on input) from that, there are likely a bunch of them for which return_code/errorlevel will only differ because of the read(stdin) result (error for invalid handle vs empty buffer for NUL). of course, there are a few others (CHOICE coming to mind first) where defining input is a must have. Passing input handle in CreateProcess will block input, hence identify the potential culprits. But this will be another pain on test writer to understand why the test times out. Perhaps, having a first shot at which tests trigger reading from input could help in deciding on best approach. how to you get the issues with find? AFAICS the cmd.exe tests only use the form with a path specified, which shouldn't read from stdin. so maybe there's something else lurking here
I completely agree find shouldn't touch stdin unless arguments say it should
but it does ```c #include <windows.h>
int main(int argc, char** argv) { SECURITY_ATTRIBUTES sa = { sizeof(sa), 0, TRUE }; STARTUPINFOA si = { sizeof(si) }; PROCESS_INFORMATION pi; HANDLE stdout = GetStdHandle(STD_OUTPUT_HANDLE); HANDLE nul = CreateFileA("NUL", GENERIC_READ, FILE_SHARE_WRITE|FILE_SHARE_READ, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); DWORD dummy; WriteFile(stdout, "first\r\n", strlen("first\r\n"), &dummy, NULL); si.dwFlags = STARTF_USESTDHANDLES; si.hStdInput = NULL; si.hStdOutput = stdout; si.hStdError = stdout; CreateProcessA(NULL, "cmd /c find /?", NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); WaitForSingleObject(pi.hProcess, INFINITE); WriteFile(stdout, "second\r\n", strlen("second\r\n"), &dummy, NULL); si.hStdInput = nul; CreateProcessA(NULL, "cmd /c find /?", NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); WaitForSingleObject(pi.hProcess, INFINITE); WriteFile(stdout, "done\r\n", strlen("done\r\n"), &dummy, NULL); return 0; } ```
On Fri Jul 18 10:19:43 2025 +0000, Alfred Agrell wrote:
I completely agree find shouldn't touch stdin unless arguments say it should but it does
#include <windows.h> int main(int argc, char** argv) { SECURITY_ATTRIBUTES sa = { sizeof(sa), 0, TRUE }; STARTUPINFOA si = { sizeof(si) }; PROCESS_INFORMATION pi; HANDLE stdout = GetStdHandle(STD_OUTPUT_HANDLE); HANDLE nul = CreateFileA("NUL", GENERIC_READ, FILE_SHARE_WRITE|FILE_SHARE_READ, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); DWORD dummy; WriteFile(stdout, "first\r\n", strlen("first\r\n"), &dummy, NULL); si.dwFlags = STARTF_USESTDHANDLES; si.hStdInput = NULL; si.hStdOutput = stdout; si.hStdError = stdout; CreateProcessA(NULL, "cmd /c find /?", NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); WaitForSingleObject(pi.hProcess, INFINITE); WriteFile(stdout, "second\r\n", strlen("second\r\n"), &dummy, NULL); si.hStdInput = nul; CreateProcessA(NULL, "cmd /c find /?", NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); WaitForSingleObject(pi.hProcess, INFINITE); WriteFile(stdout, "done\r\n", strlen("done\r\n"), &dummy, NULL); return 0; }
on wine, this returns unknown argument twice. On native, the second call returns usage, first returns a blank string.
On Fri Jul 18 10:41:25 2025 +0000, Alfred Agrell wrote:
on wine, this returns unknown argument twice. On native, the second call returns usage, first returns a blank string.
thanks for the small repro
yes, look like native `find` bails out when some std handles are invalid (tried with a large file to search into, and it returns immediately).
I tend to agree that if could avoid to redirect find's input; that would be nice
one option that we could consider is to pass as stdin the handle to a file filled with '\n' and check at the end of test if file position hasn't changed (that would detect rogue read operations that should be set inside the tests) and giving test writer some insight of what happens). hoping that find doesn't actually read from it.
On Fri Jul 18 13:34:59 2025 +0000, eric pouech wrote:
thanks for the small repro yes, look like native `find` bails out when some std handles are invalid (tried with a large file to search into, and it returns immediately). I tend to agree that if could avoid to redirect find's input; that would be nice one option that we could consider is to pass as stdin the handle to a file filled with '\n' and check at the end of test if file position hasn't changed (that would detect rogue read operations that should be set inside the tests) and giving test writer some insight of what happens). hoping that find doesn't actually read from it.
you'd hope so, but that'd be too easy, wouldn't it ```c #include <windows.h> #include <stdio.h>
int main(int argc, char** argv) { SECURITY_ATTRIBUTES sa = { sizeof(sa), 0, TRUE }; STARTUPINFOA si = { sizeof(si) }; PROCESS_INFORMATION pi; HANDLE random_file = CreateFileA("C:\Windows\win.ini", GENERIC_READ, FILE_SHARE_WRITE|FILE_SHARE_READ, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); DWORD dummy; SetFilePointer(random_file, 1, NULL, FILE_BEGIN); si.dwFlags = STARTF_USESTDHANDLES; si.hStdInput = random_file; si.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); si.hStdError = GetStdHandle(STD_OUTPUT_HANDLE); CreateProcessA(NULL, "find.exe "aaa" file-that-doesnt-exist.txt", NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); WaitForSingleObject(pi.hProcess, INFINITE); printf("fp=%lu\n", SetFilePointer(random_file, 0, NULL, FILE_CURRENT)); return 0; } ``` wine - fp=1 of course native - fp=92
oh well, twas worth trying, would've been an improvement if it worked
On Fri Jul 18 14:27:41 2025 +0000, Alfred Agrell wrote:
you'd hope so, but that'd be too easy, wouldn't it
#include <windows.h> #include <stdio.h> int main(int argc, char** argv) { SECURITY_ATTRIBUTES sa = { sizeof(sa), 0, TRUE }; STARTUPINFOA si = { sizeof(si) }; PROCESS_INFORMATION pi; HANDLE random_file = CreateFileA("C:\\Windows\\win.ini", GENERIC_READ, FILE_SHARE_WRITE|FILE_SHARE_READ, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); DWORD dummy; SetFilePointer(random_file, 1, NULL, FILE_BEGIN); si.dwFlags = STARTF_USESTDHANDLES; si.hStdInput = random_file; si.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); si.hStdError = GetStdHandle(STD_OUTPUT_HANDLE); CreateProcessA(NULL, "find.exe \"aaa\" file-that-doesnt-exist.txt", NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); WaitForSingleObject(pi.hProcess, INFINITE); printf("fp=%lu\n", SetFilePointer(random_file, 0, NULL, FILE_CURRENT)); return 0; }wine - fp=1 of course native - fp=92 oh well, twas worth trying, would've been an improvement if it worked
thanks for looking into it...
looks like find is handling pipes differently from files
``` diff --git a/programs/cmd/tests/batch.c b/programs/cmd/tests/batch.c index 03a05f34215..f066f0617ce 100644 --- a/programs/cmd/tests/batch.c +++ b/programs/cmd/tests/batch.c @@ -111,7 +111,9 @@ static BOOL run_cmd(const char *res_name, const char *cmd_data, DWORD cmd_size) char command_cmd[] = "test.cmd", command_bat[] = "test.bat"; char *command; STARTUPINFOA si = {sizeof(si)}; + SIZE_T input_watch_dog_size = 0; PROCESS_INFORMATION pi; + HANDLE rpipe, wpipe; HANDLE file,fileerr; DWORD size; BOOL bres; @@ -143,23 +145,42 @@ static BOOL run_cmd(const char *res_name, const char *cmd_data, DWORD cmd_size) if(fileerr == INVALID_HANDLE_VALUE) return FALSE;
+ if ((bres = CreatePipe(&rpipe, &wpipe, &sa, 0))) + { + char buffer[64]; + memset(buffer, 26 /* ctrl-Z */, sizeof(buffer)); + bres = WriteFile(wpipe, buffer, sizeof(buffer), &size, NULL) && + size == sizeof(buffer); + input_watch_dog_size = sizeof(buffer); + } + else rpipe = wpipe = INVALID_HANDLE_VALUE; + ok(bres, "Couldn't create pipe for input\n"); + si.dwFlags = STARTF_USESTDHANDLES; + si.hStdInput = rpipe; si.hStdOutput = file; si.hStdError = fileerr; bres = CreateProcessA(NULL, command, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi); ok(bres, "CreateProcess failed: %lu\n", GetLastError()); - if(!bres) { - DeleteFileA("test.out"); - return FALSE; - }
- WaitForSingleObject(pi.hProcess, INFINITE); - CloseHandle(pi.hThread); - CloseHandle(pi.hProcess); + if (bres) + { + WaitForSingleObject(pi.hProcess, INFINITE); + ok(PeekNamedPipe(rpipe, NULL, 0, NULL, &size, NULL), "Couldn't access read pipe\n"); + ok(size == input_watch_dog_size, "Invalid test, some command reads from stdin (%lu <> %Iu)\n", size, input_watch_dog_size); + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); + } + else + DeleteFileA("test.out"); CloseHandle(file); CloseHandle(fileerr); + CloseHandle(rpipe); + CloseHandle(wpipe); + DeleteFileA(command); - return TRUE; + + return bres; }
static DWORD map_file(const char *file_name, const char **ret)
``` technically speaking: - this seems to work as expected. (contains other cleanup:s to handle error cases from CreateProcess) - without your changes to `DATE` & `TIME` tests, it fails on Wine triggering the test about stdin being read. - with your changes to `DATE` & `TIME` tests, all tests pass on Wine. - On native (only tested Win10), some errors (about `DEL` & friends) are reported (as find has now a valid stdin)
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
eric pouech (＠epo)