[PATCH 1/2] kernel32/path: Allow renaming a file/directory to a different capitalization of itself
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
Currently Wine simply finds the existing foobar and thus "changes" it from foobar to foobar, which is a no-op and wrong. This is especially annoying with file managers when trying to rename files to different capitalization...
The __wine_wcstoumbs is taken from the similar __wine_init_codepages layout used in locale.c. I've had to do it this way and export it because accessing the ntdll_wcstoumbs internal function is required here to build the filename in its original capitalization, untouched.
I am aware that the extern function declaration may not be acceptable, but I made it similar to __wine_init_codepages. I'd appreciate some feedback on how to do this better.
dlls/kernel32/path.c | 69 +++++++++++++++++++++++++++++++++++++++---- dlls/ntdll/ntdll.spec | 1 + 2 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/dlls/kernel32/path.c b/dlls/kernel32/path.c index cf1c768..cda55f5 100644 --- a/dlls/kernel32/path.c +++ b/dlls/kernel32/path.c @@ -1136,6 +1136,16 @@ static BOOL is_same_file(HANDLE h1, HANDLE h2) return ret; }
+static ULONG get_handle_attr(HANDLE handle) +{ + FILE_BASIC_INFORMATION info; + IO_STATUS_BLOCK io; + NTSTATUS status; + + status = NtQueryInformationFile(handle, &io, &info, sizeof(info), FileBasicInformation); + return status != STATUS_SUCCESS ? 0 : info.FileAttributes; +} + /************************************************************************** * CopyFileW (KERNEL32.@) */ @@ -1310,6 +1320,9 @@ BOOL WINAPI MoveFileWithProgressW( LPCWSTR source, LPCWSTR dest, LPPROGRESS_ROUTINE fnProgress, LPVOID param, DWORD flag ) { + extern int __wine_wcstoumbs(DWORD flags, const WCHAR* src, int srclen, char* dst, + int dstlen, const char* defchar, int *used); + FILE_BASIC_INFORMATION info; UNICODE_STRING nt_name; OBJECT_ATTRIBUTES attr; @@ -1317,6 +1330,7 @@ BOOL WINAPI MoveFileWithProgressW( LPCWSTR source, LPCWSTR dest, NTSTATUS status; HANDLE source_handle = 0, dest_handle = 0; ANSI_STRING source_unix, dest_unix; + BOOL same_file = FALSE; DWORD options;
TRACE("(%s,%s,%p,%p,%04x)\n", @@ -1369,18 +1383,22 @@ BOOL WINAPI MoveFileWithProgressW( LPCWSTR source, LPCWSTR dest, SetLastError( ERROR_PATH_NOT_FOUND ); goto error; } - options = FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT; + options = FILE_SYNCHRONOUS_IO_NONALERT; if (flag & MOVEFILE_WRITE_THROUGH) options |= FILE_WRITE_THROUGH; status = NtOpenFile( &dest_handle, GENERIC_READ | GENERIC_WRITE | SYNCHRONIZE, &attr, &io, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, options ); if (status == STATUS_SUCCESS) /* destination exists */ { - if (!(flag & MOVEFILE_REPLACE_EXISTING)) + ULONG attr = 0; + + if (!(flag & MOVEFILE_REPLACE_EXISTING) || + ((attr = get_handle_attr(dest_handle)) & FILE_ATTRIBUTE_DIRECTORY)) { - if (!is_same_file( source_handle, dest_handle )) + if (!(same_file = is_same_file( source_handle, dest_handle ))) { - SetLastError( ERROR_ALREADY_EXISTS ); + SetLastError( (attr & FILE_ATTRIBUTE_DIRECTORY) + ? ERROR_ACCESS_DENIED : ERROR_ALREADY_EXISTS ); RtlFreeUnicodeString( &nt_name ); goto error; } @@ -1390,6 +1408,7 @@ BOOL WINAPI MoveFileWithProgressW( LPCWSTR source, LPCWSTR dest, SetLastError( ERROR_ACCESS_DENIED ); goto error; } + else same_file = is_same_file( source_handle, dest_handle );
NtClose( dest_handle ); dest_handle = NULL; @@ -1402,13 +1421,53 @@ BOOL WINAPI MoveFileWithProgressW( LPCWSTR source, LPCWSTR dest, }
status = wine_nt_to_unix_file_name( &nt_name, &dest_unix, FILE_OPEN_IF, FALSE ); - RtlFreeUnicodeString( &nt_name ); if (status != STATUS_SUCCESS && status != STATUS_NO_SUCH_FILE) { SetLastError( RtlNtStatusToDosError(status) ); + RtlFreeUnicodeString( &nt_name ); goto error; }
+ /* when it's renaming to the same file, preserve the case sensitivity of the last + component, so that renaming a\b\c\foobar to a\b\c\Foobar works correctly like + on Windows, but note that due to hard links, the paths must be exactly identical */ + if (same_file && source_unix.Length == dest_unix.Length && + !memcmp(source_unix.Buffer, dest_unix.Buffer, dest_unix.Length)) + { + size_t ntpathlen, pathlen; + int num, used_default; + char *tmp; + + /* find the '' from the end */ + for (ntpathlen = nt_name.Length; nt_name.Buffer[--ntpathlen] != '\';) {} + ntpathlen++; + + /* build it from path + filename (the latter converted from nt_name) */ + num = __wine_wcstoumbs(0, nt_name.Buffer + ntpathlen, nt_name.Length - ntpathlen, + NULL, 0, NULL, &used_default); + if (num > 0 && !used_default) + { + /* find the '/' from the end */ + for (pathlen = dest_unix.Length; --pathlen > 0;) + if (dest_unix.Buffer[pathlen] == '/') { pathlen++; break; } + + tmp = dest_unix.Buffer; + if (source_unix.MaximumLength < pathlen + num + 1) + tmp = RtlReAllocateHeap(GetProcessHeap(), 0, tmp, pathlen + num + 1); + if (tmp) + { + dest_unix.Buffer = tmp; + dest_unix.Length = pathlen + num; + dest_unix.MaximumLength = pathlen + num + 1; + dest_unix.Buffer[dest_unix.Length] = '\0'; + + __wine_wcstoumbs(0, nt_name.Buffer + ntpathlen, nt_name.Length - ntpathlen, + dest_unix.Buffer + pathlen, num, NULL, NULL); + } + } + } + RtlFreeUnicodeString( &nt_name ); + /* now perform the rename */
if (rename( source_unix.Buffer, dest_unix.Buffer ) == -1) diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 6062cfb..2299d9f 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -1515,6 +1515,7 @@
# Codepages @ cdecl __wine_init_codepages(ptr ptr ptr) +@ cdecl __wine_wcstoumbs(long wstr long ptr long str ptr) ntdll_wcstoumbs
# signal handling @ cdecl __wine_set_signal_handler(long ptr)
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/kernel32/tests/file.c | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 33b17aa..f8d36ba 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -1895,6 +1895,7 @@ static void test_MoveFileA(void) char tempdir[MAX_PATH]; char source[MAX_PATH], dest[MAX_PATH]; static const char prefix[] = "pfx"; + WIN32_FIND_DATAA find_data; HANDLE hfile; HANDLE hmapfile; DWORD ret; @@ -1964,9 +1965,48 @@ static void test_MoveFileA(void) ok(ret, "MoveFileA: failed, error %d\n", GetLastError());
lstrcatA(tempdir, "Remove Me"); + + /* test renaming a file "Remove Me" to itself but in lowercase "me" */ + lstrcpyA(source, tempdir); + tempdir[lstrlenA(tempdir) - 2] = 'm'; + + hfile = CreateFileA(source, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, 0, 0); + ok(hfile != INVALID_HANDLE_VALUE, "failed to create %s\n", source); + CloseHandle(hfile); + + ret = MoveFileA(source, tempdir); + ok(ret, "MoveFileA: failed, error %d\n", GetLastError()); + + hfile = FindFirstFileA(tempdir, &find_data); + ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); + if (hfile != INVALID_HANDLE_VALUE) + { + ok(!lstrcmpA(strrchr(tempdir, '\') + 1, find_data.cFileName), + "MoveFile failed to change caps on same file: got %s\n", find_data.cFileName); + } + CloseHandle(hfile); + + ret = DeleteFileA(tempdir); + ok(ret, "DeleteFileA: error %d\n", GetLastError()); + + /* now test a directory from "Remove me" to uppercase "Me" */ ret = CreateDirectoryA(tempdir, NULL); ok(ret == TRUE, "CreateDirectoryA failed\n");
+ lstrcpyA(source, tempdir); + tempdir[lstrlenA(tempdir) - 2] = 'M'; + ret = MoveFileA(source, tempdir); + ok(ret, "MoveFileA: failed, error %d\n", GetLastError()); + + hfile = FindFirstFileA(tempdir, &find_data); + ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); + if (hfile != INVALID_HANDLE_VALUE) + { + ok(!lstrcmpA(strrchr(tempdir, '\') + 1, find_data.cFileName), + "MoveFile failed to change caps on same directory: got %s\n", find_data.cFileName); + } + CloseHandle(hfile); + lstrcpyA(source, dest); lstrcpyA(dest, tempdir); lstrcatA(dest, "\wild?.*");
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=49121
Your paranoid android.
=== debian9 (32 bit report) ===
kernel32: file.c:1915: Test failed: MoveFileA: failed, error 3 file.c:1978: Test failed: MoveFileA: failed, error 3 file.c:1984: Test failed: MoveFile failed to change caps on same file: got Remove Me
=== debian9 (32 bit French report) ===
kernel32: file.c:1915: Test failed: MoveFileA: failed, error 3 file.c:1978: Test failed: MoveFileA: failed, error 3 file.c:1984: Test failed: MoveFile failed to change caps on same file: got Remove Me
=== debian9 (32 bit Japanese:Japan report) ===
kernel32: file.c:1915: Test failed: MoveFileA: failed, error 3 file.c:1978: Test failed: MoveFileA: failed, error 3 file.c:1984: Test failed: MoveFile failed to change caps on same file: got Remove Me
=== debian9 (32 bit Chinese:China report) ===
kernel32: file.c:1915: Test failed: MoveFileA: failed, error 3 file.c:1978: Test failed: MoveFileA: failed, error 3 file.c:1984: Test failed: MoveFile failed to change caps on same file: got Remove Me
=== debian9 (32 bit WoW report) ===
kernel32: file.c:1915: Test failed: MoveFileA: failed, error 3 file.c:1978: Test failed: MoveFileA: failed, error 3 file.c:1984: Test failed: MoveFile failed to change caps on same file: got Remove Me
=== debian9 (64 bit WoW report) ===
kernel32: file.c:1918: Test failed: MoveFileA: unexpected error 2 file.c:1925: Test failed: failed to open source file file.c:1928: Test failed: WriteFile error 6 file.c:1932: Test failed: CreateFileMapping: error 87 file.c:1936: Test failed: MoveFileA: expected ERROR_SHARING_VIOLATION, got 2 file.c:1947: Test failed: failed to open source file file.c:1950: Test failed: CreateFileMapping: error 87 file.c:1954: Test failed: MoveFileA: expected ERROR_SHARING_VIOLATION, got 2 file.c:1965: Test failed: MoveFileA: failed, error 2 file.c:1981: Test failed: FindFirstFileA: failed, error 2 file.c:1990: Test failed: DeleteFileA: error 2 file.c:2043: Test failed: DeleteFileA: error 2
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=49120
Your paranoid android.
=== debian9 (32 bit report) ===
kernel32: file.c:1914: Test failed: MoveFileA: failed, error 3
=== debian9 (32 bit Chinese:China report) ===
kernel32: file.c:1914: Test failed: MoveFileA: failed, error 3
=== debian9 (32 bit WoW report) ===
kernel32: file.c:1914: Test failed: MoveFileA: failed, error 3
=== debian9 (64 bit WoW report) ===
kernel32: file.c:1917: Test failed: MoveFileA: unexpected error 2 file.c:1924: Test failed: failed to open source file file.c:1927: Test failed: WriteFile error 6 file.c:1931: Test failed: CreateFileMapping: error 87 file.c:1935: Test failed: MoveFileA: expected ERROR_SHARING_VIOLATION, got 2 file.c:1946: Test failed: failed to open source file file.c:1949: Test failed: CreateFileMapping: error 87 file.c:1953: Test failed: MoveFileA: expected ERROR_SHARING_VIOLATION, got 2 file.c:1964: Test failed: MoveFileA: failed, error 2 file.c:2003: Test failed: DeleteFileA: error 2
On 3/11/19 8:50 AM, Gabriel Ivăncescu wrote:
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
Currently Wine simply finds the existing foobar and thus "changes" it from foobar to foobar, which is a no-op and wrong. This is especially annoying with file managers when trying to rename files to different capitalization...
The __wine_wcstoumbs is taken from the similar __wine_init_codepages layout used in locale.c. I've had to do it this way and export it because accessing the ntdll_wcstoumbs internal function is required here to build the filename in its original capitalization, untouched.
I am aware that the extern function declaration may not be acceptable, but I made it similar to __wine_init_codepages. I'd appreciate some feedback on how to do this better.
Can't you just use WideCharToMultiByte() with CP_UNIXCP? Unless I'm misreading that does the same thing.
On 3/12/19 12:44 AM, Zebediah Figura wrote:
On 3/11/19 8:50 AM, Gabriel Ivăncescu wrote:
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
Currently Wine simply finds the existing foobar and thus "changes" it from foobar to foobar, which is a no-op and wrong. This is especially annoying with file managers when trying to rename files to different capitalization...
The __wine_wcstoumbs is taken from the similar __wine_init_codepages layout used in locale.c. I've had to do it this way and export it because accessing the ntdll_wcstoumbs internal function is required here to build the filename in its original capitalization, untouched.
I am aware that the extern function declaration may not be acceptable, but I made it similar to __wine_init_codepages. I'd appreciate some feedback on how to do this better.
Can't you just use WideCharToMultiByte() with CP_UNIXCP? Unless I'm misreading that does the same thing.
Hi Zeb, thanks for that, I wasn't aware about it. If it does the same thing then that's great. Also, I guess I might have done some wrong rebasing on this patch due to the testbot failures, I'll see about it as well.
-
Gabriel Ivăncescu -
Marvin -
Zebediah Figura