[PATCH v11 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 -- v11: cmd: Add wildcard expansion for the type command's arguments. cmd: Add tests for the expansion of wildcards in 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 | 115 ++++++++++++++++++++--- programs/cmd/tests/test_builtins.bat.exp | 20 ++-- programs/cmd/tests/test_builtins.cmd.exp | 20 ++-- 3 files changed, 122 insertions(+), 33 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 1c9074b75d3..ccd5f574680 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,7 @@ RETURN_CODE WCMD_type(WCHAR *args) int argno = 0; WCHAR *argN = args; BOOL writeHeaders = FALSE; + HANDLE hOut; if (param1[0] == 0x00) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOARG)); @@ -3449,31 +3452,117 @@ RETURN_CODE WCMD_type(WCHAR *args) /* Loop through all args */ return_code = NO_ERROR; + hOut = GetStdHandle(STD_OUTPUT_HANDLE); while (argN) { WCHAR *thisArg = WCMD_parameter (args, argno++, &argN, FALSE, FALSE); - HANDLE hIn, hOut; + HANDLE hIn; DWORD console_mode; 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 this_arg_len; + DWORD till_file_name_part_len; + DWORD file_name_len; + + 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; + this_arg_len = wcslen(thisArg); + + do { + WCHAR srcpath[MAX_PATH]; + + if(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { + WINE_TRACE("Skipping directories\n"); + continue; + } + + foundOnlyDirectories = FALSE; + + fileNamePart = PathFindFileNameW(thisArg); + + /* Calculate length till fileNamePart */ + till_file_name_part_len = this_arg_len - wcslen(fileNamePart); + file_name_len = wcslen(fd.cFileName); + + if ((till_file_name_part_len + file_name_len) >= MAX_PATH) { + return_code = ERROR_INVALID_FUNCTION; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_FILENAMETOOLONG), thisArg); + + FindClose(hff); + continue; + } + + StringCbCopyNW(srcpath, MAX_PATH, thisArg, till_file_name_part_len); + lstrcpyW(srcpath + till_file_name_part_len, fd.cFileName); + srcpath[till_file_name_part_len + file_name_len] = L'\0'; + + hIn = CreateFileW(srcpath, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", srcpath); + } + + WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &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, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL); + + if (hIn == INVALID_HANDLE_VALUE) { + DWORD is_dir_error; - WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + /* Store GetLastError right after CreateFileW so that it isn't overwritten */ + is_dir_error = GetLastError(); - CloseHandle (hIn); + 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; + } + + continue; + } + + if (writeHeaders) { + WCMD_output_stderr(L"\n%1\n\n\n", thisArg); + } + + WCMD_copy_loop(hIn, hOut, GetConsoleMode(hIn, &console_mode) || GetConsoleMode(hOut, &console_mode)); + + 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
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? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145197
with this recent change, I have done the following: 1. Called `GetConsoleMode(hOut)` only once, right after getting `hOut` before the while loop storing it in `hOutAscii`. I changed the ordering while calling `WCMD_copy_loop` as well, to `hOutAscii || GetConsoleMode(hIn)`. The rationale is to avoid the (relatively)costly ioctl calls involved in `GetConsoleMode` by returning early if `hOutAscii` is set. 2. Added a check for `fileNamePart = PathFindFileNameW(thisArg)` which only computes it once. 3. Moved the shared flags to before the while loop, as this just seemed like unnecessary computation(should a const specifier be used here?) 4. Added a log statement that prints the string after wildcard expansion. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11094#note_145323
participants (2)
-
Barath Kannan -
Barath Kannan (＠barath_kannan)