Skip to content

Conversation

muellerj2
Copy link
Contributor

Fixes #3571 for <filesystem> and <experimental/filesystem>.

The fix for <experimental/filesystem> affects the redist because it changes the implementation of _Equivalent in filesys.cpp that is exported by msvcp140.dll.

There is no additional test coverage because a reliable test for this issue would involve setting up a drive commonly producing such transient file IDs. But I verified manually with a drive shared by Remote Desktop that (a) false positives by std::filesystem::equivalent and std::experimental::filesystem::equivalent are present without this PR and (b) are gone with it. I also added comments at the top of _Equivalent and __std_fs_equivalent that reference #3571 and serve as a reminder why the file handles can't be closed before file IDs for both files have been obtained.

@muellerj2 muellerj2 requested a review from a team as a code owner November 24, 2024 18:04
@CaseyCarter CaseyCarter added bug Something isn't working affects redist Results in changes to separately compiled bits filesystem C++17 filesystem labels Nov 24, 2024
@StephanTLavavej StephanTLavavej self-assigned this Nov 25, 2024
@StephanTLavavej StephanTLavavej removed their assignment Dec 2, 2024
@StephanTLavavej
Copy link
Member

Thanks, this is awesome! 😻 I pushed cosmetic changes.

@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 0053a14 into microsoft:main Dec 5, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks again for figuring out how to fix this runtime correctness bug! 📄 🟰 📜

@muellerj2 muellerj2 deleted the fix-filesystem-equivalent branch December 5, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects redist Results in changes to separately compiled bits bug Something isn't working filesystem C++17 filesystem
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

<filesystem>: equivalent() gives false positives on Remote Desktop shared folders
4 participants