When creating explorer favorite use more specific windows functions #5690
Conversation
|
|
src/libsync/utility_win.cpp
Outdated
| QLibrary kernel32Lib("shell32.dll"); | ||
| if(kernel32Lib.load()) | ||
| { | ||
| SHGetKnownFolderPathPtr = (SHGetKnownFolderPathFun) kernel32Lib.resolve("SHGetKnownFolderPath"); |
There was a problem hiding this comment.
This function seems to be supported since Windows Vista, but we require Windows 7 at least. Would it be possible to use normal dynamic linking to access it rather than doing a manual symbol lookup?
There was a problem hiding this comment.
Last time i tried this i had errors with direct lookup, but maybe because was testing also XP clients. If you say that Windows 7 is the minimum supported, i can try rewriting it
There was a problem hiding this comment.
@remixtj I'd be great if you could try rewriting to use normal dynamic linking. Feel free to ping me if you need someone to test on Win7.
src/libsync/utility_win.cpp
Outdated
|
|
||
| /* Use new WINAPI functions */ | ||
| wchar_t *path = NULL; | ||
| if(SHGetKnownFolderPath(FOLDERID_Links, 0, NULL, &path) == S_OK) { |
There was a problem hiding this comment.
From the documentation about the path argument:
The calling process is responsible for freeing this resource once it is no longer needed by calling CoTaskMemFree.
Could you verify or fix it?
src/libsync/utility_win.cpp
Outdated
| linkName = QDir(links).filePath(folderDir.dirName() + QLatin1String(".lnk")); | ||
| } | ||
| qDebug() << Q_FUNC_INFO << " creating link from " << linkName << " to " << folder; | ||
| CoTaskMemFree(path); |
There was a problem hiding this comment.
This will crash if path is not null and yet not pointing to a valid allocation, which can happen if SHGetKnownFolderPath doesn't touch it. You could either move CoTaskMemFree into the if(){}, or initialize path to NULL.
|
Argh...
The CI (and our build servers) might not be able to cross-build this... Are you testing your patch on Windows with MinGW, or are you using MSVC? |
|
Hi, i'm compiling on mingw32-w64 on opensuse. Maybe my environment is not much up to date, but works. |
|
That's perfect then, that's our official toolchain. Thanks for the patch and for fixing the build, I'll merge it 👍 |
Fixes #2719