[PATCH v4 1/2] ntdll: Allow renaming a file/directory to a different casing of itself.
Gabriel Ivăncescu
gabrielopcode at gmail.com
Wed Apr 21 06:40:46 CDT 2021
On 21/04/2021 00:02, Rémi Bernon wrote:
> On 4/20/21 5:04 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 `casinglen` which is
>> the
>> length of this data.
>>
>> dlls/ntdll/unix/file.c | 59 ++++++++++++++++++++------
>> server/fd.c | 95 +++++++++++++++++++++++++++++-------------
>> server/protocol.def | 2 +
>> 3 files changed, 114 insertions(+), 42 deletions(-)
>>
>> diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
>> index 488f748..0cf1a7f 100644
>> --- a/dlls/ntdll/unix/file.c
>> +++ b/dlls/ntdll/unix/file.c
>> @@ -2800,6 +2800,31 @@ void init_files(void)
>> }
>> +/***********************************************************************
>> + * get_nt_file_name_unix_casing
>> + *
>> + * Get the Unix file name with the same casing as the NT file name
>> (without path).
>> + */
>> +static unsigned int get_nt_file_name_unix_casing(const
>> OBJECT_ATTRIBUTES *attr, char **casing)
>> +{
>> + const WCHAR *filename = attr->ObjectName->Buffer +
>> attr->ObjectName->Length / sizeof(WCHAR);
>> + int len, casinglen = 0;
>> +
>> + /* NT name may not be NUL terminated; look for last \ character */
>> + for (; filename != attr->ObjectName->Buffer; filename--)
>> + if (filename[-1] == '\\')
>> + break;
>> + len = attr->ObjectName->Buffer + attr->ObjectName->Length /
>> sizeof(WCHAR) - filename;
>> +
>> + if ((*casing = malloc( len * 3 )))
>> + {
>> + casinglen = ntdll_wcstoumbs( filename, len, *casing, len * 3,
>> TRUE );
>> + if (casinglen < 0) casinglen = 0;
>> + }
>> + return casinglen;
>> +}
>> +
>> +
>>
>> /******************************************************************************
>>
>> * get_dos_device
>> *
>> @@ -4521,8 +4546,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>> handle, IO_STATUS_BLOCK *io,
>> {
>> FILE_RENAME_INFORMATION *info = ptr;
>> UNICODE_STRING name_str, redir;
>> + char *unix_name, *casing;
>> OBJECT_ATTRIBUTES attr;
>> - char *unix_name;
>> name_str.Buffer = info->FileName;
>> name_str.Length = info->FileNameLength;
>> @@ -4533,20 +4558,25 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>> handle, IO_STATUS_BLOCK *io,
>> io->u.Status = nt_to_unix_file_name( &attr, &unix_name,
>> FILE_OPEN_IF );
>> if (io->u.Status == STATUS_SUCCESS || io->u.Status ==
>> STATUS_NO_SUCH_FILE)
>> {
>> + unsigned int casinglen =
>> get_nt_file_name_unix_casing( &attr, &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->link = FALSE;
>> - req->replace = info->ReplaceIfExists;
>> + req->handle = wine_server_obj_handle( handle );
>> + req->rootdir = wine_server_obj_handle(
>> attr.RootDirectory );
>> + req->namelen = attr.ObjectName->Length;
>> + req->casinglen = 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 );
>> }
>> SERVER_END_REQ;
>> free( unix_name );
>> + free( casing );
>> }
>> free( redir.Buffer );
>> }
>
>
> I think it'd be better to handle the casing allocation even before
> nt_to_unix_file_name, and set Status in case of failure.
>
> Then I'm not sure how likely ntdll_wcstoumbs is expected to fail, but in
> that case it should probably just use strrchr( unix_name, '/' ) or
> unix_name, to have a nicer fallback.
>
>> @@ -4558,8 +4588,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>> handle, IO_STATUS_BLOCK *io,
>> {
>> FILE_LINK_INFORMATION *info = ptr;
>> UNICODE_STRING name_str, redir;
>> + char *unix_name, *casing;
>> OBJECT_ATTRIBUTES attr;
>> - char *unix_name;
>> name_str.Buffer = info->FileName;
>> name_str.Length = info->FileNameLength;
>> @@ -4570,20 +4600,25 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>> handle, IO_STATUS_BLOCK *io,
>> io->u.Status = nt_to_unix_file_name( &attr, &unix_name,
>> FILE_OPEN_IF );
>> if (io->u.Status == STATUS_SUCCESS || io->u.Status ==
>> STATUS_NO_SUCH_FILE)
>> {
>> + unsigned int casinglen =
>> get_nt_file_name_unix_casing( &attr, &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->link = TRUE;
>> - req->replace = info->ReplaceIfExists;
>> + req->handle = wine_server_obj_handle( handle );
>> + req->rootdir = wine_server_obj_handle(
>> attr.RootDirectory );
>> + req->namelen = attr.ObjectName->Length;
>> + req->casinglen = 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 );
>> }
>> SERVER_END_REQ;
>> free( unix_name );
>> + free( casing );
>> }
>> free( redir.Buffer );
>> }
>
> Here too you could set casing to be strrchr( unix_name, '/' ) + 1 or
> unix_name. So it wouldn't need an allocation. It's also probably not
> worth keeping the separate helper.
>
> (My OCD also tells me that using "caselen" instead of "casinglen" in the
> request would keep things aligned and save you the re-identation, but
> that probably doesn't matter much ;))
>
Alright, sounds good to me, and with such fallback I guess the helper
won't really save much code.
>> diff --git a/server/fd.c b/server/fd.c
>> index 481e9a8..0b4c0a1 100644
>> --- a/server/fd.c
>> +++ b/server/fd.c
>> @@ -2488,11 +2488,14 @@ static void set_fd_disposition( struct fd *fd,
>> int unlink )
>> /* set new name for the fd */
>> static void set_fd_name( struct fd *fd, struct fd *root, const char
>> *nameptr, data_size_t len,
>> - struct unicode_str nt_name, int create_link,
>> int replace )
>> + const char *casing, data_size_t casinglen,
>> struct unicode_str nt_name,
>> + int create_link, int replace )
>> {
>> + char *name, *filename;
>> + int different_casing;
>> struct inode *inode;
>> struct stat st, st2;
>> - char *name;
>> + size_t filenamelen;
>> if (!fd->inode || !fd->unix_name)
>> {
>> @@ -2533,50 +2536,79 @@ static void set_fd_name( struct fd *fd, struct
>> fd *root, const char *nameptr, da
>> goto failed;
>> }
>> + filename = strrchr( name, '/' );
>> + filename = filename ? filename + 1 : name;
>> + filenamelen = strlen( filename );
>> + different_casing = casinglen && (filenamelen != casinglen ||
>> memcmp( filename, casing, casinglen ));
>> +
>
> I think with the fallback above the casinglen == 0 check is probably not
> very useful here?
>
> Also I would do the name realloc here, or even earlier, right after
> combined_name, simply checking if casinglen > filenamelen, to make sure
> we can simply copy casing to filename later.
>
Good idea. Should I treat the casinglen == 0 as an input error then?
(now I'm using it as fallback)
>> if (!stat( name, &st ))
>> {
>> if (!fstat( fd->unix_fd, &st2 ) && st.st_ino == st2.st_ino
>> && st.st_dev == st2.st_dev)
>> {
>> if (create_link && !replace) set_error(
>> STATUS_OBJECT_NAME_COLLISION );
>> - free( name );
>> - return;
>> - }
>> -
>> - if (!replace)
>> - {
>> - set_error( STATUS_OBJECT_NAME_COLLISION );
>> - goto failed;
>> + if (!different_casing)
>> + {
>> + free( name );
>> + return;
>> + }
>> }
>> -
>> - /* can't replace directories or special files */
>> - if (!S_ISREG( st.st_mode ))
>> + else
>> {
>> - set_error( STATUS_ACCESS_DENIED );
>> - goto failed;
>> - }
>> + if (!replace)
>> + {
>> + set_error( STATUS_OBJECT_NAME_COLLISION );
>> + goto failed;
>> + }
>> - /* can't replace an opened file */
>> - if ((inode = get_inode( st.st_dev, st.st_ino, -1 )))
>> - {
>> - int is_empty = list_empty( &inode->open );
>> - release_object( inode );
>> - if (!is_empty)
>> + /* can't replace directories or special files */
>> + if (!S_ISREG( st.st_mode ))
>> {
>> set_error( STATUS_ACCESS_DENIED );
>> goto failed;
>> }
>> +
>> + /* can't replace an opened file */
>> + if ((inode = get_inode( st.st_dev, st.st_ino, -1 )))
>> + {
>> + int is_empty = list_empty( &inode->open );
>> + release_object( inode );
>> + if (!is_empty)
>> + {
>> + set_error( STATUS_ACCESS_DENIED );
>> + goto failed;
>> + }
>> + }
>> +
>> + /* link() expects that the target doesn't exist */
>> + /* rename() cannot replace files with directories */
>> + if (create_link || S_ISDIR( st2.st_mode ) ||
>> different_casing)
>> + {
>> + if (unlink( name ))
>> + {
>> + file_set_error();
>> + goto failed;
>> + }
>> + }
>> }
>> + }
>> - /* link() expects that the target doesn't exist */
>> - /* rename() cannot replace files with directories */
>> - if (create_link || S_ISDIR( st2.st_mode ))
>> + if (different_casing)
>> + {
>> + size_t pathlen = filename - name;
>> + if (filenamelen < casinglen)
>> {
>> - if (unlink( name ))
>> + char *new_name = mem_alloc( pathlen + casinglen + 1 );
>> + if (!new_name)
>> {
>> - file_set_error();
>> + set_error( STATUS_NO_MEMORY );
>> goto failed;
>> }
>> + memcpy( new_name, name, pathlen );
>> + free( name );
>> + name = new_name;
>> }
>> + memcpy(name + pathlen, casing, casinglen);
>> + name[pathlen + casinglen] = 0;
>> }
>
> And so this whole if could be something like that instead:
>
> memcpy( filename, casing, caselen );
> filename[caselen] = 0;
>
Should I not check for different_casing here? It should be the most
common case after all and avoid a copy.
>> if (create_link)
>> @@ -2879,16 +2911,19 @@ DECL_HANDLER(set_fd_disp_info)
>> /* set fd name information */
>> DECL_HANDLER(set_fd_name_info)
>> {
>> + const char *filename, *casing;
>> struct fd *fd, *root_fd = NULL;
>> struct unicode_str nt_name;
>> - if (req->namelen > get_req_data_size())
>> + if (req->namelen > get_req_data_size() || get_req_data_size() -
>> req->namelen < req->casinglen)
>> {
>> set_error( STATUS_INVALID_PARAMETER );
>> return;
>> }
>> nt_name.str = get_req_data();
>> nt_name.len = (req->namelen / sizeof(WCHAR)) * sizeof(WCHAR);
>> + casing = (const char *)get_req_data() + req->namelen;
>> + filename = casing + req->casinglen;
>> if (req->rootdir)
>> {
>> @@ -2902,8 +2937,8 @@ DECL_HANDLER(set_fd_name_info)
>> if ((fd = get_handle_fd_obj( current->process, req->handle, 0 )))
>> {
>> - set_fd_name( fd, root_fd, (const char *)get_req_data() +
>> req->namelen,
>> - get_req_data_size() - req->namelen, nt_name,
>> req->link, req->replace );
>> + set_fd_name( fd, root_fd, filename, (const char
>> *)get_req_data() + get_req_data_size() - filename,
>> + casing, req->casinglen, nt_name, req->link,
>> req->replace );
>> release_object( fd );
>> }
>> if (root_fd) release_object( root_fd );
>> diff --git a/server/protocol.def b/server/protocol.def
>> index 9ea6967..8560469 100644
>> --- a/server/protocol.def
>> +++ b/server/protocol.def
>> @@ -3531,9 +3531,11 @@ struct handle_info
>> obj_handle_t handle; /* handle to a file or directory */
>> obj_handle_t rootdir; /* root directory */
>> data_size_t namelen; /* length of NT name in bytes */
>> + data_size_t casinglen; /* length of the casing filename */
>> int link; /* link instead of renaming */
>> int replace; /* replace an existing file? */
>> VARARG(name,unicode_str,namelen); /* NT name */
>> + VARARG(casing,string,casinglen); /* new file name's actual
>> casing (without path) */
>> VARARG(filename,string); /* new file name */
>> @END
>>
>
>
More information about the wine-devel
mailing list