- Wine-GitLab - list.winehq.org

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Also, as Huw mentioned above, it looks like you still didn't fully address the comments from @nsivov that were pointed out [here](https://www.winehq.org/mailman3/hyperkitty/list/wine-devel(a)winehq.o…. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121819

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + > + if (!SUCCEEDED(IShellLinkW_QueryInterface(shelllink, &IID_IPersistFile, (LPVOID*)&persistfile))) > + { > + ERR("couldn't get IPersistFile interface\n"); > + IShellLinkW_Release(shelllink); > + return; > + } > + > + do { > + WCHAR *link_filename; > + > + if (counter == 1) > + { > + static const WCHAR *fmt = L"%s - %s.lnk"; > + int length = wcslen(filename) + wcslen(shortcutW) + 7; /* length of the format*/ > + link_filename = malloc((length + 1) * sizeof(WCHAR)); Should you check if malloc returns null? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121810

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + WCHAR *link_filename; > + > + if (counter == 1) > + { > + static const WCHAR *fmt = L"%s - %s.lnk"; > + int length = wcslen(filename) + wcslen(shortcutW) + 7; /* length of the format*/ > + link_filename = malloc((length + 1) * sizeof(WCHAR)); > + wsprintfW(link_filename, fmt, filename, shortcutW); > + } > + else > + { > + static const WCHAR *fmt = L"%s - %s (%s).lnk"; > + int length = wcslen(filename) + wcslen(shortcutW) + 10; /* length of the format*/ > + WCHAR counter_string[34]; > + wsprintfW(counter_string, L"%i", counter); > + length += wcslen(counter_string); `wsprintfW` also returns the length of the string by the way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121809

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + { > + ERR("SetIDList failed\n"); > + return; > + } > + _ILSimpleGetTextW(This->apidl[0], (LPVOID)filename, MAX_PATH); > + length = wcslen(filename) + wcslen(shortcutW); > + if (length > MAX_PATH) > + { > + ERR("Path maximum length exceeded (%i>%i)\n", length, MAX_PATH); > + IShellLinkW_Release(shelllink); > + return; > + } > + > + SHGetPathFromIDListW(This->pidl, root); > + > + if (!SUCCEEDED(IShellLinkW_QueryInterface(shelllink, &IID_IPersistFile, (LPVOID*)&persistfile))) In these failure cases, it's probably possible to write some tests to see what `HRESULT` should be returned. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121808

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + do { > + WCHAR *link_filename; > + > + if (counter == 1) > + { > + static const WCHAR *fmt = L"%s - %s.lnk"; > + int length = wcslen(filename) + wcslen(shortcutW) + 7; /* length of the format*/ > + link_filename = malloc((length + 1) * sizeof(WCHAR)); > + wsprintfW(link_filename, fmt, filename, shortcutW); > + } > + else > + { > + static const WCHAR *fmt = L"%s - %s (%s).lnk"; > + int length = wcslen(filename) + wcslen(shortcutW) + 10; /* length of the format*/ > + WCHAR counter_string[34]; > + wsprintfW(counter_string, L"%i", counter); I think we can just allocate a sufficient amount for `link_filename` and add the counter directly in the `wsprintfW` call below. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121805

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + ILFree(full_pidl); > + if (FAILED(hr)) > + { > + ERR("SetIDList failed\n"); > + return; > + } > + _ILSimpleGetTextW(This->apidl[0], (LPVOID)filename, MAX_PATH); > + length = wcslen(filename) + wcslen(shortcutW); > + if (length > MAX_PATH) > + { > + ERR("Path maximum length exceeded (%i>%i)\n", length, MAX_PATH); > + IShellLinkW_Release(shelllink); > + return; > + } > + > + SHGetPathFromIDListW(This->pidl, root); In other places, we check the return value of this function. Not sure if we should do that here as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121807

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + ITEMIDLIST *full_pidl; > + HRESULT hr; > + int length; > + int counter = 1; > + > + static const WCHAR *shortcutW = L"Shortcut"; > + > + if (FAILED(IShellLink_Constructor(NULL, &IID_IShellLinkW, (LPVOID*)&shelllink))) > + { > + ERR("couldn't create ShellLink object\n"); > + return; > + } > + full_pidl = ILCombine(This->pidl, This->apidl[0]); > + if (!full_pidl) > + { > + ERR("Out of Memory\n"); Why don't we just return an `HRESULT` here and return that from `ItemMenu_InvokeCommand`, instead of the verbose ERRs? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121806

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + return; > + } > + > + PathCombineW(lnkfile, root, link_filename); > + free(link_filename); > + counter++; > + > + if (PathFileExistsW(lnkfile)) > + { > + hr = HRESULT_FROM_WIN32(ERROR_FILE_EXISTS); > + } > + else > + { > + ERR("attempting to write link to %s\n", wine_dbgstr_w(lnkfile)); > + hr = IPersistFile_Save(persistfile, lnkfile, FALSE); > + ERR("ARIC %#lx\n",hr); Debug statements -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121804

1 0

Re: [PATCH v8 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK
by Sven Baars (＠sbaars) Nov. 13, 2025

Nov. 13, 2025

Sven Baars (@sbaars) commented about dlls/shell32/shlview_cmenu.c: > + full_pidl = ILCombine(This->pidl, This->apidl[0]); > + if (!full_pidl) > + { > + ERR("Out of Memory\n"); > + return; > + } > + > + hr = IShellLinkW_SetIDList(shelllink, full_pidl); > + ILFree(full_pidl); > + if (FAILED(hr)) > + { > + ERR("SetIDList failed\n"); > + return; > + } > + _ILSimpleGetTextW(This->apidl[0], (LPVOID)filename, MAX_PATH); > + length = wcslen(filename) + wcslen(shortcutW); This looks like a duplicate check. There is another one in the loop below. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_121803

1 0

[PATCH 0/4] MR9429: Avoid a few compiler warnings.
by Bernhard Übelacker Nov. 13, 2025

Nov. 13, 2025

Gitlab CI uses `--enable-werror` for its builds, but to improve backtraces with my recent address sanitizer experiments I am using these flags `-g -O1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls -fno-inline-functions` with both, gcc at the unix side and llvm-mingw at the pe side. Therefore these warnings would turn into errors. This four patches are helping gcc at the unix side. Would these patches be acceptable? Should they be separate merge requests? <details> <summary>Details</summary> ``` ccache gcc -m64 -c -o tools/winebuild/utils.o .../wine/tools/winebuild/utils.c \ -Itools/winebuild -I.../wine/tools/winebuild -Iinclude \ -I.../wine/include -D__WINESRC__ -DWINE_UNIX_LIB -Wall -pipe \ -fcf-protection=none -fvisibility=hidden -fno-stack-protector -fno-strict-aliasing \ -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers -Winit-self -Wlogical-op \ -Wno-packed-not-aligned -Wpointer-arith -Wshift-overflow=2 -Wstrict-prototypes -Wtype-limits \ -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wno-misleading-indentation -gdwarf-4 -Werror \ -g -O1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls -fno-inline-functions .../wine/tools/winebuild/utils.c: In function 'find_tool': .../wine/tools/winebuild/utils.c:230:8: error: 'file' may be used uninitialized [-Werror=maybe-uninitialized] 230 | if (!file && cc_command.count) file = find_clang_tool( cc_command, name ); | ^ .../wine/tools/winebuild/utils.c:214:17: note: 'file' was declared here 214 | const char *file; | ^~~~ cc1: all warnings being treated as errors make: *** [Makefile:610992: tools/winebuild/utils.o] Error 1 ccache gcc -m64 -c -o dlls/ntdll/unix/file.o .../wine/dlls/ntdll/unix/file.c -Idlls/ntdll \ -I.../wine/dlls/ntdll -Iinclude \ -I.../wine/include -D__WINESRC__ -DLTC_NO_PROTOTYPES \ -DLTC_SOURCE -D_NTSYSTEM_ -D_ACRTIMP= -DWINBASEAPI= -DWINE_UNIX_LIB \ -I/usr/include/x86_64-linux-gnu -Wall -pipe -fcf-protection=none -fvisibility=hidden \ -fno-stack-protector -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body \ -Wignored-qualifiers -Winit-self -Wlogical-op -Wno-packed-not-aligned -Wpointer-arith \ -Wshift-overflow=2 -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla \ -Wwrite-strings -Wno-misleading-indentation -gdwarf-4 -Werror -fPIC -fasynchronous-unwind-tables \ -g -O1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls -fno-inline-functions .../wine/dlls/ntdll/unix/file.c: In function 'NtReadFile': .../wine/dlls/ntdll/unix/file.c:5758:23: error: 'avail_mode' may be used uninitialized [-Werror=maybe-uninitialized] 5758 | if (total && avail_mode) | ~~~~~~^~~~~~~~~~~~~ .../wine/dlls/ntdll/unix/file.c:5755:18: note: 'avail_mode' was declared here 5755 | BOOL avail_mode; | ^~~~~~~~~~ cc1: all warnings being treated as errors make: *** [Makefile:318598: dlls/ntdll/unix/file.o] Error 1 ccache gcc -m64 -c -o dlls/sane.ds/unixlib.o .../wine/dlls/sane.ds/unixlib.c -Idlls/sane.ds \ -I.../wine/dlls/sane.ds -Iinclude \ -I.../wine/include -D__WINESRC__ -DWINE_UNIX_LIB -Wall -pipe \ -fcf-protection=none -fvisibility=hidden -fno-stack-protector -fno-strict-aliasing \ -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers -Winit-self -Wlogical-op \ -Wno-packed-not-aligned -Wpointer-arith -Wshift-overflow=2 -Wstrict-prototypes -Wtype-limits \ -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wno-misleading-indentation -gdwarf-4 -Werror \ -fPIC -fasynchronous-unwind-tables -g -O1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls -fno-inline-functions .../wine/dlls/sane.ds/unixlib.c: In function 'read_data': .../wine/dlls/sane.ds/unixlib.c:332:8: error: 'status' may be used uninitialized [-Werror=maybe-uninitialized] 332 | if (status == SANE_STATUS_EOF) return TWCC_SUCCESS; | ^ .../wine/dlls/sane.ds/unixlib.c:321:17: note: 'status' was declared here 321 | SANE_Status status; | ^~~~~~ cc1: all warnings being treated as errors make: *** [Makefile:366624: dlls/sane.ds/unixlib.o] Error 1 ccache gcc -m64 -c -o server/directory.o .../wine/server/directory.c -Iserver \ -I.../wine/server -Iinclude \ -I.../wine/include -D__WINESRC__ -DWINE_UNIX_LIB -Wall -pipe \ -fcf-protection=none -fvisibility=hidden -fno-stack-protector -fno-strict-aliasing \ -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers -Winit-self -Wlogical-op \ -Wno-packed-not-aligned -Wpointer-arith -Wshift-overflow=2 -Wstrict-prototypes -Wtype-limits \ -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wno-misleading-indentation -gdwarf-4 -Werror \ -g -O1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fno-optimize-sibling-calls -fno-inline-functions .../wine/server/directory.c: In function 'create_session': .../wine/server/directory.c:327:44: error: 'snprintf' output may be truncated before the last format character [-Werror=format-truncation=] 327 | snprintf( id_strA, sizeof(id_strA), "%u", id ); | ^ .../wine/server/directory.c:327:5: note: 'snprintf' output between 2 and 11 bytes into a destination of size 10 327 | snprintf( id_strA, sizeof(id_strA), "%u", id ); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [Makefile:607457: server/directory.o] Error 1 ``` </details> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9429

2 7