[PATCH v13 0/2] MR11094: cmd: Add wildcard expansion for type command.
Fixes: https://bugs.winehq.org/show_bug.cgi?id=59816 Signed-off-by: Barath Kannan barathrk11@gmail.com -- v13: cmd: Add wildcard expansion for the type command's arguments. https://gitlab.winehq.org/wine/wine/-/merge_requests/11094
From: Barath Kannan <barathrk11@gmail.com> Signed-off-by: Barath Kannan <barathrk11@gmail.com> --- programs/cmd/tests/test_builtins.bat | 14 ++++++++++---- programs/cmd/tests/test_builtins.bat.exp | 11 +++++++++-- programs/cmd/tests/test_builtins.cmd | 14 ++++++++++---- programs/cmd/tests/test_builtins.cmd.exp | 11 +++++++++-- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/programs/cmd/tests/test_builtins.bat b/programs/cmd/tests/test_builtins.bat index 4c3d0e7107e..5903c9967f8 100644 --- a/programs/cmd/tests/test_builtins.bat +++ b/programs/cmd/tests/test_builtins.bat @@ -99,16 +99,22 @@ call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\" rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo +mkdir bar +mkdir spam echo a > fileA echo b > fileB call :setError 666 & (type &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- -call :setError 666 & (type i\dont\exist\at\all.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -cd .. && rd /q /s foo - +call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type file* idontexistatall.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type idontexistatall.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type i\dont\exist\at\all.txt file*&&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +cd .. +del foo\file* +call :setError 666 & (type foo\* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +rd /q /s foo echo --- success/failure for COPY command mkdir foo & cd foo echo a > fileA diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 36e14d5be91..0738a87f6b8 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -67,11 +67,18 @@ SUCCESS 1024 FAILURE 1 SUCCESS 0 FAILURE 1 +--- @todo_wine@a@space@ @todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@--- FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index f65a301d914..f02fa85e0ad 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -682,16 +682,22 @@ call :setError 666 & (start /B /WAIT /d "%FOO_PATH%" cmd /s /c "if /I \"%%cd%%\" rd /q /s foo echo --- success/failure for TYPE command mkdir foo & cd foo +mkdir bar +mkdir spam echo a > fileA echo b > fileB call :setError 666 & (type &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type NUL &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) call :setError 666 & (type i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) echo --- -call :setError 666 & (type i\dont\exist\at\all.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) -cd .. && rd /q /s foo - +call :setError 666 & (type file* i\dont\exist\at\all.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type file* idontexistatall.txt &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type idontexistatall.txt file* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +call :setError 666 & (type i\dont\exist\at\all.txt file*&&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +cd .. +del foo\file* +call :setError 666 & (type foo\* &&echo SUCCESS !errorlevel!||echo FAILURE !errorlevel!) +rd /q /s foo echo --- success/failure for COPY command mkdir foo & cd foo echo a > fileA diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0f6d9bdb494..79defed5cdb 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -586,11 +586,18 @@ SUCCESS 1024 FAILURE 1 SUCCESS 0 FAILURE 1 +--- @todo_wine@a@space@ @todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@--- FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@a@space@ +@todo_wine@b@space@ +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 +@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11094
From: Barath Kannan <barathrk11@gmail.com> Signed-off-by: Barath Kannan <barathrk11@gmail.com> --- programs/cmd/builtins.c | 131 ++++++++++++++++++++--- programs/cmd/tests/test_builtins.bat.exp | 20 ++-- programs/cmd/tests/test_builtins.cmd.exp | 20 ++-- 3 files changed, 136 insertions(+), 35 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 1c9074b75d3..927df03af14 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -30,6 +30,8 @@ #include "wcmd.h" #include <shellapi.h> +#include <shlwapi.h> +#include <strsafe.h> #include "winternl.h" #include "winioctl.h" #include "ddk/ntifs.h" @@ -3439,6 +3441,10 @@ RETURN_CODE WCMD_type(WCHAR *args) int argno = 0; WCHAR *argN = args; BOOL writeHeaders = FALSE; + HANDLE hOut; + BOOL hOutAscii; + DWORD console_mode; + DWORD fileShareAccessFlags; if (param1[0] == 0x00) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOARG)); @@ -3447,33 +3453,128 @@ RETURN_CODE WCMD_type(WCHAR *args) if (param2[0] != 0x00) writeHeaders = TRUE; - /* Loop through all args */ return_code = NO_ERROR; + hOut = GetStdHandle(STD_OUTPUT_HANDLE); + hOutAscii = GetConsoleMode(hOut, &console_mode); + fileShareAccessFlags = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; + + /* Loop through all args */ while (argN) { WCHAR *thisArg = WCMD_parameter (args, argno++, &argN, FALSE, FALSE); - HANDLE hIn, hOut; - DWORD console_mode; + HANDLE hIn; if (!argN) break; WINE_TRACE("type: Processing arg '%s'\n", wine_dbgstr_w(thisArg)); - hIn = CreateFileW(thisArg, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, NULL); - if (hIn == INVALID_HANDLE_VALUE) { - WCMD_print_error (); - WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); - return errorlevel = ERROR_INVALID_FUNCTION; - } - hOut = GetStdHandle(STD_OUTPUT_HANDLE); - if (writeHeaders) { - WCMD_output_stderr(L"\n%1\n\n\n", thisArg); + if (wcspbrk(thisArg, L"*?")) + { + BOOL foundOnlyDirectories = TRUE; + WIN32_FIND_DATAW fd; + HANDLE hff = INVALID_HANDLE_VALUE; + WCHAR *fileNamePart; + DWORD till_file_name_part_len; + DWORD full_path_len; + WCHAR srcpath[MAX_PATH]; + + hff = FindFirstFileW(thisArg, &fd); + + if (hff == INVALID_HANDLE_VALUE) { + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + return ERROR_INVALID_FUNCTION; + } + + writeHeaders = TRUE; + fileNamePart = NULL; + srcpath[0] = L'\0'; + + do { + if(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { + WINE_TRACE("Skipping directories\n"); + continue; + } + + foundOnlyDirectories = FALSE; + + /* Compute fileNamePart and till_file_name_part_len only for the first time */ + if (fileNamePart == NULL) { + fileNamePart = PathFindFileNameW(thisArg); + till_file_name_part_len = thisArg - fileNamePart; + } + + /* Calculate length till fileNamePart */ + full_path_len = till_file_name_part_len + wcslen(fd.cFileName); + + if (full_path_len >= MAX_PATH) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_FILENAMETOOLONG), thisArg); + + continue; + } + + /* Copy to srcpath only if it is empty */ + if (!srcpath[0]) { + StringCbCopyNW(srcpath, MAX_PATH, thisArg, till_file_name_part_len); + } + + lstrcpyW(srcpath + till_file_name_part_len, fd.cFileName); + srcpath[full_path_len] = L'\0'; + + WINE_TRACE("type: Expanded arg to'%s'\n", wine_dbgstr_w(srcpath)); + + hIn = CreateFileW(srcpath, GENERIC_READ, fileShareAccessFlags, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", srcpath); + } + + WCMD_copy_loop(hIn, hOut, hOutAscii || GetConsoleMode(hIn, &console_mode)); + + CloseHandle (hIn); + + } while (FindNextFileW(hff, &fd) != 0); + + FindClose (hff); + + if (foundOnlyDirectories) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_FILENOTFOUND), thisArg); + } } + else + { + hIn = CreateFileW(thisArg, GENERIC_READ, fileShareAccessFlags, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + if (hIn == INVALID_HANDLE_VALUE) { + DWORD is_dir_error; + + /* Store GetLastError right after CreateFileW so that it isn't overwritten */ + is_dir_error = GetLastError(); + + return_code = ERROR_INVALID_FUNCTION; + WCMD_print_error (); + WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); + + /* Invalid directory path; Return immediately */ + if(is_dir_error == ERROR_PATH_NOT_FOUND) { + return errorlevel = ERROR_INVALID_FUNCTION; + } - WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + continue; + } + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", thisArg); + } + + WCMD_copy_loop(hIn, hOut, hOutAscii || GetConsoleMode(hIn, &console_mode)); - CloseHandle (hIn); + CloseHandle (hIn); + } } return errorlevel = return_code; diff --git a/programs/cmd/tests/test_builtins.bat.exp b/programs/cmd/tests/test_builtins.bat.exp index 0738a87f6b8..77ca71c8778 100644 --- a/programs/cmd/tests/test_builtins.bat.exp +++ b/programs/cmd/tests/test_builtins.bat.exp @@ -68,17 +68,17 @@ FAILURE 1 SUCCESS 0 FAILURE 1 --- -@todo_wine@a@space@ -@todo_wine@b@space@ +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +FAILURE 1 FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 79defed5cdb..dc6420e1432 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -587,17 +587,17 @@ FAILURE 1 SUCCESS 0 FAILURE 1 --- -@todo_wine@a@space@ -@todo_wine@b@space@ +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +a@space@ +b@space@ +FAILURE 1 +FAILURE 1 FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@a@space@ -@todo_wine@b@space@ -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 -@todo_wine@FAILURE 1 --- success/failure for COPY command FAILURE 1 SUCCESS 0 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11094
On Fri Jul 10 12:35:31 2026 +0000, Barath Kannan wrote:
In this recent push I removed the following block from the wildcard present in `thisArg` path. My reasoning for this is simply that if the path doesn't exist, `FindFirstFileW` catches it beforehand, so there is no real need to validate the result of `CreateFileW` . Is there any case where this fails though? Where somehow FindFirstFileW detects a file but maybe it gets deleted somehow before CreateFileW is called? ``` if (hIn == INVALID_HANDLE_VALUE) { DWORD is_dir_error; /* Store GetLastError right after CreateFileW so that it isn't overwritten */ is_dir_error = GetLastError(); return_code = ERROR_INVALID_FUNCTION; WCMD_print_error (); WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), srcpath); FindClose(hff);
/* Invalid directory path; Return immediately */ if(is_dir_error == ERROR_PATH_NOT_FOUND) { return errorlevel = ERROR_INVALID_FUNCTION; } continue; } ``` I also moved the ``` hOut = GetStdHandle(STD_OUTPUT_HANDLE); ``` line to before the start of the while loop as STD_OUTPUT won't change throughout the program's execution. it should be fine to factor the ERROR_PATH_NOT_FOUND test only after FindFirstFileW
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145376
On Fri Jul 10 12:47:14 2026 +0000, Barath Kannan wrote:
just attempted to rerun the pipelines so that failing tests would pass(ironcally 3 unrelated tests failed as opposed to the initial one that I attempted to fix). is it a requirement for all tests to pass for an MR to be accepted? it's accepted to have failing tests in CI when one can clearly tag as unrelated to the commit
(that could be other unrelated MR failing at the same spots...) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145377
you should also test here for path not found (should also stop the loop around args as it's done with no wildcarded arg) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145378
we tend to avoid Hungary notation and camel case names... even more when the prefix is wrong -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145379
On Fri Jul 10 14:34:20 2026 +0000, eric pouech wrote:
it's accepted to have failing tests in CI when one can clearly tag as unrelated to the commit (that could be other unrelated MR failing at the same spots...) alright, fair enoug
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145383
On Fri Jul 10 14:44:28 2026 +0000, eric pouech wrote:
we tend to avoid Hungary notation and camel case names... even more when the prefix is wrong Changed the newly added variables to use snake case.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145384
participants (3)
-
Barath Kannan -
Barath Kannan (＠barath_kannan)
-
eric pouech (＠epo)