fix two problems in the Dir module: #2674
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asomers
force-pushed
the
readdir_r
branch
3 times, most recently
from
ff8e8bc to
9386cb9
Compare
asomers
changed the title
asomers
force-pushed
the
readdir_r
branch
2 times, most recently
from
7e5602c to
4644f30
Compare
readdir_r does not work well on systems where {NAME_MAX} can vary. The main
reason to use it is for thread-safety. However, POSIX 1003.1-2024 Issue 8
makes readdir_r obsolete and clarifies that readdir must be thread-safe except
when concurrent calls are made to the same directory stream. So all
applications should prefer it now. The original rationale for Nix using
readdir_r[^1], in addition to thread safety, was that the Rust standard library
used it. But that is no longer the case. So Nix should switch to plain
readdir.
The second problem is that on some operating systems, libc::dirent is an
open-ended structure where the last field (d_name) has a size of 1 byte,
but additional data follow the end of the structure. Nix currently
relies on copying the libc::dirent structure, which can't possibly work
on those platforms. Fix that bug by copying the d_name field to an
owned CString.
[^1]: f9ebcb7 , from 2018
Fixes nix-rust#2673
Fixes nix-rust#1783
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
readdir_r does not work well on systems where {NAME_MAX} can vary. The main reason to use it is for thread-safety. However, POSIX 1003.1-2024 Issue 8 makes readdir_r obsolete and clarifies that readdir must be thread-safe except when concurrent calls are made to the same directory stream. So all applications should prefer it now. The original rationale for Nix using readdir_r1, in addition to thread safety, was that the Rust standard library used it. But that is no longer the case. So Nix should switch to plain readdir.
The second problem is that on some operating systems, libc::dirent is an
open-ended structure where the last field (d_name) has a size of 1 byte,
but additional data follow the end of the structure. Nix currently
relies on copying the libc::dirent structure, which can't possibly work
on those platforms. Fix that bug by copying the d_name field to an
owned CString.
What does this PR do
Checklist:
CONTRIBUTING.mdFootnotes
f9ebcb7 , from 2018 ↩