[PATCH v4 1/2] ntdll: Allow renaming a file/directory to a different casing of itself.
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(a)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 ); } @@ -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 ); } 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 )); + 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; } 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 -- 2.30.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/kernel32/tests/file.c | 64 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 5deed96..dfa69db 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -1954,6 +1954,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; @@ -2023,9 +2024,72 @@ 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); + + /* test renaming another file "Remove Be" to "Remove Me", which replaces the existing "Remove me" */ + tempdir[lstrlenA(tempdir) - 2] = 'B'; + + hfile = CreateFileA(tempdir, 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", tempdir); + CloseHandle(hfile); + + ret = MoveFileA(tempdir, source); + ok(!ret, "MoveFileA: expected failure\n"); + ok(GetLastError() == ERROR_ALREADY_EXISTS, "MoveFileA: expected ERROR_ALREADY_EXISTS, got %d\n", GetLastError()); + ret = MoveFileExA(tempdir, source, MOVEFILE_REPLACE_EXISTING); + ok(ret, "MoveFileExA: failed, error %d\n", GetLastError()); + + tempdir[lstrlenA(tempdir) - 2] = 'm'; + + hfile = FindFirstFileA(tempdir, &find_data); + ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); + if (hfile != INVALID_HANDLE_VALUE) + { + ok(!lstrcmpA(strrchr(source, '\\') + 1, find_data.cFileName), + "MoveFile failed to change caps on existing target 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?.*"); -- 2.30.0
Hi, While running your changed tests, 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=89075 Your paranoid android. === debiant2 (32 bit Chinese:China report) === ntdll: om.c:2305: Test failed: got 87
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(a)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 ;))
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.
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;
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
-- Rémi Bernon <rbernon(a)codeweavers.com>
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(a)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
On 4/21/21 1:40 PM, Gabriel Ivăncescu wrote:
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(a)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)
I don't think it should ever happen? I mean, even with the strrchr fallback above you would have caselen == 0 only if the last path element is empty, which means an empty filename in unix_name? Handling caselen == 0 in the common case should work, it would just do 0-size comparisons and copies. If it's not valid input then I expect the code to be handling the case already somehow, possibly because rename / link would fail, and they would fail here all the same?
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.
I don't think it's useful and it makes the logic more complicated than it need to be. We have already read the whole filename to compare the casing, so I don't think the copy would take any noticeable time, even with long file names.
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
-- Rémi Bernon <rbernon(a)codeweavers.com>
participants (3)
-
Gabriel Ivăncescu -
Marvin -
Rémi Bernon