[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 07:42:49 CDT 2021


On 22/04/2021 13:34, Gabriel Ivăncescu wrote:
> 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.

So actually, I found out a way to simplify it and also handle links 
properly without sending caselen == 0, I hope it will be fine I'll send 
it shortly. If this is still too complicated or has some edge cases I'm 
out of ideas without complicating the ntdll code too much...



More information about the wine-devel mailing list