KEMBAR78
`filesystem.cpp`: Avoid loading `GetTempPath2W` for OneCore by StephanTLavavej · Pull Request #5565 · microsoft/STL · GitHub
Skip to content

filesystem.cpp: Avoid loading GetTempPath2W for OneCore #5565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 14, 2025
Merged

filesystem.cpp: Avoid loading GetTempPath2W for OneCore #5565

merged 1 commit into from
Jun 14, 2025

Conversation

@StephanTLavavej
Copy link
Member

GetModuleHandleW is a "desktop apps only" API. We avoid calling it for OneCore:

STL/stl/src/winapisupp.cpp

Lines 179 to 194 in 7841cf8

#if defined(_ONECORE)
// All APIs are statically available, and we can't call GetModuleHandleW().
#else // ^^^ defined(_ONECORE) / !defined(_ONECORE) vvv
static int __cdecl initialize_pointers() noexcept {
HINSTANCE hKernel32 = GetModuleHandleW(L"kernel32.dll");
_Analysis_assume_(hKernel32);
// Note that GetTempPath2W is defined as of Windows 10 Build 20348 (a server release) or Windows 11,
// but there is no "_WIN32_WINNT_WIN11" constant, so we will always dynamically load it
STOREFUNCTIONPOINTER(hKernel32, GetTempPath2W);
return 0;
}

and so we can't load GetTempPath2W for OneCore and must always use the GetTempPathW fallback instead:

STL/stl/src/winapisupp.cpp

Lines 164 to 175 in 7841cf8

extern "C" _Success_(return > 0 && return < BufferLength) DWORD __stdcall __crtGetTempPath2W(
_In_ DWORD BufferLength, _Out_writes_to_opt_(BufferLength, return +1) LPWSTR Buffer) noexcept {
#if !defined(_ONECORE)
// use GetTempPath2W if it is available (only on Windows 11+)...
IFDYNAMICGETCACHEDFUNCTION(GetTempPath2W) {
return pfGetTempPath2W(BufferLength, Buffer);
}
#endif // ^^^ !defined(_ONECORE) ^^^
// ...otherwise use GetTempPathW.
return GetTempPathW(BufferLength, Buffer);
}

winapisupp.cpp is built into the STL's DLL, and the non-dllexported __crtGetTempPath2W is used by filesys.cpp in the DLL, implementing the non-Standard/highly-deprecated <experimental/filesystem>.

For Standard <filesystem>, we inject filesystem.cpp into the STL's import LIB, but we weren't similarly guarding GetModuleHandleW. (This is the only other call in the STL.) Somehow, nobody has hissed about <filesystem> being incompatible with UWP apps, but I suspect maybe nobody happened to be calling temp_directory_path() in their UWP apps, and so the linker discarded this unused machinery from the import lib. In any event, we should fix the code to be consistent and avoid potential problems for UWP.

@StephanTLavavej StephanTLavavej requested a review from a team as a code owner June 1, 2025 19:59
@StephanTLavavej StephanTLavavej added bug Something isn't working filesystem C++17 filesystem labels Jun 1, 2025
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Jun 1, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in STL Code Reviews Jun 1, 2025
@StephanTLavavej StephanTLavavej mentioned this pull request May 29, 2025
Open
@YexuanXiao
Copy link
Contributor

I suspect UWP apps prefer using WinRT over STL to obtain this information because the STL lacks support for access permissions and lacks APIs related to application data folders.

DHowett
DHowett reviewed Jun 2, 2025
View reviewed changes
davidmrdavid
davidmrdavid approved these changes Jun 11, 2025
View reviewed changes
@StephanTLavavej StephanTLavavej moved this from Final Review to Merging in STL Code Reviews Jun 11, 2025
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej added enhancement Something can be improved and removed bug Something isn't working labels Jun 14, 2025
@StephanTLavavej
Copy link
Member Author

I'm going to recharacterize this as a consistency enhancement instead of a bugfix. @amyw-msft informed me that the banned API restrictions were lifted back in 2020, and while certain APIs still can't be physically called (e.g. user32 UI), there are no deployment time blocks for common Win32 APIs. In the future I'll try to eradicate all of the OneCore checks, but for now I'll merge this.

@StephanTLavavej StephanTLavavej merged commit 487e081 into microsoft:main Jun 14, 2025
48 checks passed
@github-project-automation github-project-automation bot moved this from Merging to Done in STL Code Reviews Jun 14, 2025
@StephanTLavavej StephanTLavavej deleted the onecore-twocore-redcore-bluecore branch June 14, 2025 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidmrdavid davidmrdavid davidmrdavid approved these changes

+1 more reviewer

@DHowett DHowett DHowett left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement Something can be improved filesystem C++17 filesystem

Projects

STL Code Reviews
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@StephanTLavavej @YexuanXiao @DHowett @davidmrdavid