[PATCH v3 0/3] MR5737: Draft: kernelbase: Use ERROR_DIRECTORY for FindFirstFileExW on invalid name.
When `FindFirstFileA` is called with `<path>/<file>/*` (where file is expected to be a directory), Windows uses the `ERROR_DIRECTORY` error.
This patch changes Wine's implementation to match Windows. This fixes a crash in Unity of Command II.
-- v3: server: Don't always return STATUS_OBJECT_NAME_INVALID on ENOTDIR.
From: Brendan McGrath bmcgrath@codeweavers.com
--- dlls/kernel32/tests/file.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 02625140702..9e5c1e4c594 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -2715,6 +2715,7 @@ static void test_FindFirstFileA(void) char buffer[5] = "C:\"; char buffer2[100]; char nonexistent[MAX_PATH]; + BOOL found = FALSE;
/* try FindFirstFileA on "C:" */ buffer[0] = get_windows_drive(); @@ -2746,10 +2747,32 @@ static void test_FindFirstFileA(void) ok( FindNextFileA( handle, &data ), "FindNextFile failed\n" ); ok( !strcmp( data.cFileName, ".." ), "FindNextFile should return '..' as second entry\n" ); while (FindNextFileA( handle, &data )) + { ok ( strcmp( data.cFileName, "." ) && strcmp( data.cFileName, ".." ), "FindNextFile shouldn't return '%s'\n", data.cFileName ); + if (!found && (data.dwFileAttributes == FILE_ATTRIBUTE_NORMAL || + data.dwFileAttributes == FILE_ATTRIBUTE_ARCHIVE)) + { + GetWindowsDirectoryA( buffer2, sizeof(buffer2) ); + strcat(buffer2, "\"); + strcat(buffer2, data.cFileName); + strcat(buffer2, "\*"); + found = TRUE; + } + } ok ( FindClose(handle) == TRUE, "Failed to close handle %s\n", buffer2 );
+ ok ( found, "Windows dir should not be empty\n" ); + if (found) + { + SetLastError( 0xdeadbeef ); + handle = FindFirstFileA(buffer2, &data); + err = GetLastError(); + ok ( handle == INVALID_HANDLE_VALUE, "FindFirstFile on %s should fail\n", buffer2 ); + todo_wine + ok ( err == ERROR_DIRECTORY, "Bad Error number %x\n", err ); + } + /* try FindFirstFileA on "C:\foo" */ SetLastError( 0xdeadbeaf ); if (!GetTempFileNameA( buffer, "foo", 0, nonexistent ))
From: Brendan McGrath bmcgrath@codeweavers.com
--- dlls/ntdll/tests/file.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index 9dd49c925b7..7961abad1b1 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -414,6 +414,16 @@ static void open_file_test(void) CloseHandle( handle ); pRtlFreeUnicodeString( &nameW );
+ wcscat( path, L"\" ); + pRtlDosPathNameToNtPathName_U( path, &nameW, NULL, NULL ); + status = NtOpenFile( &handle, FILE_LIST_DIRECTORY | SYNCHRONIZE, &attr, &io, + FILE_SHARE_READ | FILE_SHARE_WRITE, + FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT | FILE_OPEN_FOR_BACKUP_INTENT ); + todo_wine + ok( status == STATUS_NOT_A_DIRECTORY, "open %s failed %lx\n", wine_dbgstr_w(nameW.Buffer), status ); + CloseHandle( handle ); + pRtlFreeUnicodeString( &nameW ); + wcscat( path, L"\cmd.exe" ); pRtlDosPathNameToNtPathName_U( path, &nameW, NULL, NULL ); status = pNtOpenFile( &handle, GENERIC_READ, &attr, &io,
From: Brendan McGrath bmcgrath@codeweavers.com
Only return STATUS_OBJECT_NAME_INVALID if we're not looking for a FILE_DIRECTORY_FILE. --- dlls/kernel32/tests/file.c | 1 - dlls/ntdll/tests/file.c | 1 - server/fd.c | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 9e5c1e4c594..940b1441e19 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -2769,7 +2769,6 @@ static void test_FindFirstFileA(void) handle = FindFirstFileA(buffer2, &data); err = GetLastError(); ok ( handle == INVALID_HANDLE_VALUE, "FindFirstFile on %s should fail\n", buffer2 ); - todo_wine ok ( err == ERROR_DIRECTORY, "Bad Error number %x\n", err ); }
diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index 7961abad1b1..7d3618a3804 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -419,7 +419,6 @@ static void open_file_test(void) status = NtOpenFile( &handle, FILE_LIST_DIRECTORY | SYNCHRONIZE, &attr, &io, FILE_SHARE_READ | FILE_SHARE_WRITE, FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT | FILE_OPEN_FOR_BACKUP_INTENT ); - todo_wine ok( status == STATUS_NOT_A_DIRECTORY, "open %s failed %lx\n", wine_dbgstr_w(nameW.Buffer), status ); CloseHandle( handle ); pRtlFreeUnicodeString( &nameW ); diff --git a/server/fd.c b/server/fd.c index fc7f7be92b9..f28937466fc 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1942,7 +1942,7 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam if (fd->unix_fd == -1) { /* check for trailing slash on file path */ - if ((errno == ENOENT || errno == ENOTDIR) && name[strlen(name) - 1] == '/') + if ((errno == ENOENT || (errno == ENOTDIR && !(options & FILE_DIRECTORY_FILE))) && name[strlen(name) - 1] == '/') set_error( STATUS_OBJECT_NAME_INVALID ); else file_set_error();
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=145904
Your paranoid android.
=== w10pro64_zh_CN (64 bit report) ===
kernel32: file.c:4698: Test failed: SetFileValidData failed 87 file.c:4701: Test failed: SetFileValidData failed 87
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Marvin