[PATCH v5 1/2] ntdll: Allow renaming a file/directory to a different casing of itself.
Gabriel Ivăncescu
gabrielopcode at gmail.com
Thu Apr 22 05:34:51 CDT 2021
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 at 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.
More information about the wine-devel
mailing list