June 2022 - Wine-Devel - list.winehq.org

Re: [PATCH v2 0/1] MR243: winhttp: Fix default port corner cases in WinHttpCrackUrl(). - approved

by Alexandre Julliard (＠julliard)

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/243

June 14, 2022

Re: [PATCH v3 0/5] MR240: win32u: menu conversion - approved

by Alexandre Julliard (＠julliard)

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/240

June 14, 2022

Re: [PATCH v2 0/6] MR226: winegstreamer: Leave stream_cs when calling the async reader callbacks. - approved

by Alexandre Julliard (＠julliard)

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 14, 2022

Re: [PATCH v2 0/1] MR149: user32: Plug icon handle leak in MDI_AugmentFrameMenu (Coverity). - approved

by Alexandre Julliard (＠julliard)

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/149

June 14, 2022

Re: Gitlab experiment wrapup

by Alexandre Julliard

Gabriel Ivăncescu <gabrielopcode(a)gmail.com> writes: > Hi, > > On 14/06/2022 13:55, Alexandre Julliard wrote: >> * Some other things I like: >> - Updating status doesn't need to go through me, people can assign >> reviewers, supersede patches, etc. directly. That reduces my workload >> and improves the bus factor. Once we have figured out how to make >> testbot results reliable, we could also have maintainers merge commits >> directly. >> > > I created my first MR since the switch seems inevitable. I completely > fail at assigning a reviewer. I even tried the web UI and I can't find > any setting. > > I also tried the /assign_reviewer "quick action" on a comment and it said: > > `Could not apply assign_reviewer command.` > > Is this feature not working currently? You need to be a project member to modify reviewers. There should be a link at the top of the project page to request access. -- Alexandre Julliard julliard(a)winehq.org

June 14, 2022

Re: Gitlab experiment wrapup

by Gabriel Ivăncescu

Hi, On 14/06/2022 13:55, Alexandre Julliard wrote: > * Some other things I like: > > - Updating status doesn't need to go through me, people can assign > reviewers, supersede patches, etc. directly. That reduces my workload > and improves the bus factor. Once we have figured out how to make > testbot results reliable, we could also have maintainers merge commits > directly. > I created my first MR since the switch seems inevitable. I completely fail at assigning a reviewer. I even tried the web UI and I can't find any setting. I also tried the /assign_reviewer "quick action" on a comment and it said: `Could not apply assign_reviewer command.` Is this feature not working currently? Thanks, Gabriel

June 14, 2022

[PATCH 1/1] ntdll: Allow renaming a file/directory to a different case of itself.

by Gabriel Ivăncescu

From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Renaming a file or directory from e.g. foobar to FooBar (or any other case change) should work, like on Windows, instead of being a no-op. Clobbering an existing file must also respect the new case. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/kernel32/tests/file.c | 4 +- dlls/ntdll/tests/file.c | 4 +- dlls/ntdll/unix/file.c | 58 +++++++++++++++++++- server/fd.c | 108 ++++++++++++++++++++++++++----------- server/protocol.def | 2 + 5 files changed, 138 insertions(+), 38 deletions(-) diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index bd8d8c91b2f..9b7066f428d 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -2049,7 +2049,7 @@ static void test_MoveFileA(void) ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError()); if (hfile != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), + ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), "MoveFile failed to change casing on same file: got %s\n", find_data.cFileName); } CloseHandle(hfile); @@ -2094,7 +2094,7 @@ static void test_MoveFileA(void) ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError()); if (hfile != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), + ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName), "MoveFile failed to change casing on same directory: got %s\n", find_data.cFileName); } CloseHandle(hfile); diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index 106413a83fc..f7f103b0589 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -2315,7 +2315,7 @@ static void test_file_link_information(void) ok(handle != INVALID_HANDLE_VALUE, "FindFirstFileW: failed, error %ld\n", GetLastError()); if (handle != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpW(wcsrchr(newpath, '\\') + 1, find_data.cFileName), + ok(!lstrcmpW(wcsrchr(newpath, '\\') + 1, find_data.cFileName), "Link did not change casing on existing target file: got %s\n", wine_dbgstr_w(find_data.cFileName)); } @@ -2900,7 +2900,7 @@ static void test_file_link_information(void) ok(handle != INVALID_HANDLE_VALUE, "FindFirstFileW: failed, error %ld\n", GetLastError()); if (handle != INVALID_HANDLE_VALUE) { - todo_wine ok(!lstrcmpW(wcsrchr(oldpath, '\\') + 1, find_data.cFileName), + ok(!lstrcmpW(wcsrchr(oldpath, '\\') + 1, find_data.cFileName), "Link did not change casing on same file: got %s\n", wine_dbgstr_w(find_data.cFileName)); } diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index cc8bf0c6e82..7460d20646a 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -3594,6 +3594,52 @@ NTSTATUS WINAPI wine_unix_to_nt_file_name( const char *name, WCHAR *buffer, ULON } +/*********************************************************************** + * get_filename_case + * + * Get the unix filename, with the case from NT name's last component. + */ +static char *get_filename_case( const OBJECT_ATTRIBUTES *attr ) +{ + const WCHAR *p, *nt_filename = attr->ObjectName->Buffer; + int len = attr->ObjectName->Length / sizeof(WCHAR); + char *file_case; + + /* skip the device and prefix (allow slashes for unix namespace) */ + if (!attr->RootDirectory) + { + int pos = get_dos_prefix_len( attr->ObjectName ); + while (pos < len) + { + WCHAR c = nt_filename[pos++]; + if (c == '\\' || c == '/') break; + } + nt_filename += pos; + len -= pos; + } + + /* strip off trailing backslashes */ + for (; len; len--) + if (nt_filename[len - 1] != '\\' && nt_filename[len - 1] != '/') + break; + + /* get the last component */ + for (p = nt_filename + len; p != nt_filename; p--) + if (p[-1] == '\\' || p[-1] == '/') + break; + len -= p - nt_filename; + nt_filename = p; + + if ((file_case = malloc( len * 3 + 1 ))) + { + len = ntdll_wcstoumbs( nt_filename, len, file_case, len * 3, TRUE ); + if (len < 0 || len > MAX_DIR_ENTRY_LEN) len = 0; + file_case[len] = 0; + } + return file_case; +} + + /*********************************************************************** * get_full_path * @@ -4534,8 +4580,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, { FILE_RENAME_INFORMATION *info = ptr; UNICODE_STRING name_str, redir; + char *unix_name, *file_case; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4546,20 +4592,24 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); if (status == STATUS_SUCCESS || status == STATUS_NO_SUCH_FILE) { + file_case = get_filename_case( &attr ); 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 = file_case ? strlen( file_case ) : 0; req->link = FALSE; req->replace = info->ReplaceIfExists; wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length ); + wine_server_add_data( req, file_case, req->caselen ); wine_server_add_data( req, unix_name, strlen(unix_name) ); status = wine_server_call( req ); } SERVER_END_REQ; free( unix_name ); + free( file_case ); } free( redir.Buffer ); } @@ -4571,8 +4621,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, { FILE_LINK_INFORMATION *info = ptr; UNICODE_STRING name_str, redir; + char *unix_name, *file_case; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4583,20 +4633,24 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); if (status == STATUS_SUCCESS || status == STATUS_NO_SUCH_FILE) { + file_case = get_filename_case( &attr ); 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 = file_case ? strlen( file_case ) : 0; req->link = TRUE; req->replace = info->ReplaceIfExists; wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length ); + wine_server_add_data( req, file_case, req->caselen ); wine_server_add_data( req, unix_name, strlen(unix_name) ); status = wine_server_call( req ); } SERVER_END_REQ; free( unix_name ); + free( file_case ); } free( redir.Buffer ); } diff --git a/server/fd.c b/server/fd.c index 1b4b98b0e76..7101cec401e 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2527,11 +2527,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 *file_case, data_size_t caselen, struct unicode_str nt_name, + int create_link, int replace ) { + size_t pathlen = 0, filenamelen; struct inode *inode; struct stat st, st2; - char *name; + int different_case; + char *name, *p; if (!fd->inode || !fd->unix_name) { @@ -2565,6 +2568,30 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da name = combined_name; } + /* get path len up to last component, ignoring trailing slashes */ + for (p = name; *p;) + if (*p++ == '/' && *p) + pathlen = p - name; + + /* get filename ptr and len, without trailing slashes */ + filenamelen = p - name - pathlen; + p = name + pathlen; + while (filenamelen && p[filenamelen - 1] == '/') + filenamelen--; + + different_case = caselen && (filenamelen != caselen || memcmp( p, file_case, caselen )); + + if (filenamelen < caselen) + { + p = realloc( name, pathlen + caselen + 1 ); + if (!p) + { + set_error( STATUS_NO_MEMORY ); + goto failed; + } + name = p; + } + /* when creating a hard link, source cannot be a dir */ if (create_link && !fstat( fd->unix_fd, &st ) && S_ISDIR( st.st_mode )) { @@ -2577,47 +2604,61 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da 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 (!different_case) + { + free( name ); + return; + } - if (!replace) - { - set_error( STATUS_OBJECT_NAME_COLLISION ); - goto failed; + /* creating a link with a different case on itself renames the file */ + create_link = 0; } - - /* 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; } - } - /* link() expects that the target doesn't exist */ - /* rename() cannot replace files with directories */ - if (create_link || S_ISDIR( st2.st_mode )) - { - if (unlink( name )) + /* can't replace an opened file */ + if ((inode = get_inode( st.st_dev, st.st_ino, -1 ))) { - file_set_error(); - goto failed; + 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_case) + { + if (unlink( name )) + { + file_set_error(); + goto failed; + } } } } + if (different_case) + { + memcpy( name + pathlen, file_case, caselen ); + name[pathlen + caselen] = 0; + } + if (create_link) { if (link( fd->unix_name, name )) @@ -2959,16 +3000,19 @@ DECL_HANDLER(set_fd_disp_info) /* set fd name information */ DECL_HANDLER(set_fd_name_info) { + const char *fullname, *file_case; 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->caselen) { set_error( STATUS_INVALID_PARAMETER ); return; } nt_name.str = get_req_data(); nt_name.len = (req->namelen / sizeof(WCHAR)) * sizeof(WCHAR); + file_case = (const char *)get_req_data() + req->namelen; + fullname = file_case + req->caselen; if (req->rootdir) { @@ -2982,8 +3026,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, fullname, (const char *)get_req_data() + get_req_data_size() - fullname, + file_case, req->caselen, 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 2be1658fca2..61d72ae5551 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3559,9 +3559,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 caselen; /* length of the actual case filename */ int link; /* link instead of renaming */ int replace; /* replace an existing file? */ VARARG(name,unicode_str,namelen); /* NT name */ + VARARG(actual_case,string,caselen); /* new file name's actual case (without path) */ VARARG(filename,string); /* new file name */ @END -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/246

June 14, 2022

[PATCH 0/1] MR246: Allow renaming a file/directory to a different case of itself

by Gabriel Ivăncescu

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/246

June 14, 2022

Re: Gitlab experiment wrapup

by Alexandre Julliard

Giovanni Mascellani <gmascellani(a)codeweavers.com> writes: > Hi, > > Il 14/06/22 12:55, Alexandre Julliard ha scritto: >> - Signoffs are a bit cumbersome (Rémi). We should change the >> requirements to something better adapted to Gitlab. > > I don't remember if this point has already been made, but we're using > the Signed-off-by pseudo-header in a rather peculiar fashion. The > standard meaning of Signed-off-by is something like "I have checked > that I have the legal right to submit this patch for inclusion" (more > specifically, see [1]), while we use it as "I have checked that this > patch is technically sound". Other projects use other pseudo-headers > for the latter, like "Reviewed-by" or "Acked-by". > > [1] https://developercertificate.org/ > > Could we take the occasion to switch to something more standard? Signed-off-by is convenient because the git tools have direct support for it, but the real motivation for us was to keep track of who submitted the patch, to support sending a patch written by someone else. In that case the convention with email is to set From: to the original author, but the committer info is lost, hence the Signed-off-by. With merge requests, the information of who submitted the merge request is preserved independently of the commit contents, so the easiest option is to simply get rid of signoffs completely. Another option would be to require the submitter to explicitly approve their own MR if it contains a patch written by someone else; that would be similar to a reviewer pushing fixups and then approving the MR. It would require some mechanism to detect missing approvals though, so it may not be worth the trouble. Thoughts? -- Alexandre Julliard julliard(a)winehq.org

June 14, 2022

[PATCH 1/1] iphlpapi: Return ERROR_NO_DATA from GetIpNetTable() if no entries are found.

by Paul Gofman

From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/iphlpapi/iphlpapi_main.c | 7 +++++++ dlls/iphlpapi/tests/iphlpapi.c | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 5a4acdb9c5a..33b12361d70 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -2326,6 +2326,13 @@ DWORD WINAPI GetIpNetTable( MIB_IPNETTABLE *table, ULONG *size, BOOL sort ) } table->dwNumEntries = count; + + if (!count) + { + err = ERROR_NO_DATA; + goto err; + } + for (i = 0; i < count; i++) { MIB_IPNETROW *row = table->table + i; diff --git a/dlls/iphlpapi/tests/iphlpapi.c b/dlls/iphlpapi/tests/iphlpapi.c index c40ae3989c6..29256eb2774 100644 --- a/dlls/iphlpapi/tests/iphlpapi.c +++ b/dlls/iphlpapi/tests/iphlpapi.c @@ -388,11 +388,11 @@ static void testGetIpNetTable(void) else if (apiReturn == ERROR_INSUFFICIENT_BUFFER) { PMIB_IPNETTABLE buf = HeapAlloc(GetProcessHeap(), 0, dwSize); + memset(buf, 0xcc, dwSize); apiReturn = GetIpNetTable(buf, &dwSize, FALSE); - ok(apiReturn == NO_ERROR || - apiReturn == ERROR_NO_DATA, /* empty ARP table's okay */ - "GetIpNetTable(buf, &dwSize, FALSE) returned %ld, expected NO_ERROR\n", - apiReturn); + ok((apiReturn == NO_ERROR && buf->dwNumEntries) || (apiReturn == ERROR_NO_DATA && !buf->dwNumEntries), + "got apiReturn %lu, dwSize %lu, buf->dwNumEntries %lu.\n", + apiReturn, dwSize, buf->dwNumEntries); if (apiReturn == NO_ERROR && winetest_debug > 1) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/245

June 14, 2022