On 21/04/2021 21:41, Rémi Bernon wrote:
On 4/21/21 2:42 PM, Gabriel Ivăncescu wrote:
Renaming a file or directory from e.g. foobar to FooBar (or any other casing change) should work, like on Windows, instead of being a no-op. Clobbering an existing file must also respect the new casing.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
We just prepend the unix casing filename (without path) to the wineserver data before the actual unix filename, and a field `caselen` which is the length of this data.
dlls/ntdll/unix/file.c | 56 ++++++++++++++++++++++-- server/fd.c | 97 ++++++++++++++++++++++++++++-------------- server/protocol.def | 2 + 3 files changed, 118 insertions(+), 37 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 488f748..d8fdb27 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4520,9 +4520,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4530,17 +4532,38 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, InitializeObjectAttributes( &attr, &name_str, OBJ_CASE_INSENSITIVE, info->RootDirectory, NULL ); get_redirect( &attr, &redir ); - io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;
+ if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY;
if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( casing ); + } SERVER_START_REQ( set_fd_name_info ) { req->handle = wine_server_obj_handle( handle ); req->rootdir = wine_server_obj_handle( attr.RootDirectory ); req->namelen = attr.ObjectName->Length; + req->caselen = casinglen; req->link = FALSE; req->replace = info->ReplaceIfExists; wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length ); + wine_server_add_data( req, casing, casinglen ); wine_server_add_data( req, unix_name, strlen(unix_name) ); io->u.Status = wine_server_call( req ); } @@ -4548,6 +4571,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
This shouldn't free if the fallback path was taken.
@@ -4557,9 +4581,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_LINK_INFORMATION)) { FILE_LINK_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4567,17 +4593,38 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, InitializeObjectAttributes( &attr, &name_str, OBJ_CASE_INSENSITIVE, info->RootDirectory, NULL ); get_redirect( &attr, &redir ); - io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;
+ if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY;
if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( casing ); + } SERVER_START_REQ( set_fd_name_info ) { req->handle = wine_server_obj_handle( handle ); req->rootdir = wine_server_obj_handle( attr.RootDirectory ); req->namelen = attr.ObjectName->Length; + req->caselen = casinglen; req->link = TRUE; req->replace = info->ReplaceIfExists; wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length ); + wine_server_add_data( req, casing, casinglen ); wine_server_add_data( req, unix_name, strlen(unix_name) ); io->u.Status = wine_server_call( req ); } @@ -4585,6 +4632,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
I'm not sure we want to support a different casing here, I think there are some additional consideration to have on the server side code:
We don't want to create link of some file to another casing of itself, but instead probably rename the link to another casing if it already exists?
I think we should just use the strrchr fallback all the time here.
However, if we want to support proper casing for FileLinkInformation too, it would probably deserve a few tests first to see what this means for links.
And the implementation could then come in a different patch to make it clearer. In which case, assuming we need the same kind of logic to send the proper casing, a separate helper would indeed be useful.
In any case, I think it would also be nice (if possible?) to have the tests first in the series with todo_wine, removed later in the patch that fixes it.
Yeah, I'm doubtful it matters for links, but I'll see about the tests in any case—and with todo_wine should be cleaner I hope.
But honestly, wouldn't it just be easier to send caselen == 0 and consider that as the fallback in the wineserver? If links even turn out to not be affected by this, it would add no change to the link case at all or any complications with freeing (as you can see, I messed up above).
Basically it boils down to: is it really worth complicating the ntdll code so much just to avoid 2 lines of code on the wineserver side? I just feel like it's not adding much value to make it non-optional, but perhaps I'm missing the bigger picture.