fs: Use readdir() instead of readdir_r() on Linux and Android #92778
Conversation
This file contains 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
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
tavianator
force-pushed
the
linux-readdir-no-r
branch
from
0c5acfb
to
ea9181b
Compare
|
@bors r+ rollup |
|
📌 Commit ea9181b has been approved by |
bors
added
S-waiting-on-bors
|
Hang on, there's a bug in metadata() |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jan 11, 2022
…shtriplett fs: Use readdir() instead of readdir_r() on Linux readdir() is preferred over readdir_r() on Linux and many other platforms because it more gracefully supports long file names. Both glibc and musl (and presumably all other Linux libc implementations) guarantee that readdir() is thread-safe as long as a single DIR* is not accessed concurrently, which is enough to make a readdir()-based implementation of ReadDir safe. This implementation is already used for some other OSes including Fuchsia, Redox, and Solaris. See rust-lang#40021 for more details. Fixes rust-lang#86649.
|
@bors r- |
bors
added
S-waiting-on-author
tavianator
force-pushed
the
linux-readdir-no-r
branch
from
ea9181b
to
4839b81
Compare
|
Okay, should be fixed. |
|
@bors r+ |
|
📌 Commit 4839b81 has been approved by |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jan 14, 2022
…shtriplett fs: Use readdir() instead of readdir_r() on Linux readdir() is preferred over readdir_r() on Linux and many other platforms because it more gracefully supports long file names. Both glibc and musl (and presumably all other Linux libc implementations) guarantee that readdir() is thread-safe as long as a single DIR* is not accessed concurrently, which is enough to make a readdir()-based implementation of ReadDir safe. This implementation is already used for some other OSes including Fuchsia, Redox, and Solaris. See rust-lang#40021 for more details. Fixes rust-lang#86649.
|
@bors r- Failed in rollup: #92878 (comment) |
bors
added
S-waiting-on-author
tavianator
force-pushed
the
linux-readdir-no-r
branch
from
4839b81
to
64c21ae
Compare
tavianator
changed the title
|
I switched Android over too, and split up the commits a bit. I would test the Android build locally but I have no idea how. Is there documentation for building Rust for Android or should I just try to follow the CI script? |
|
To test android, it needs the NDK. There is a script here for getting it, but I would just use the docker image. That can be done with |
|
Thanks! I ran the library tests in that Docker image and they passed. |
|
☔ The latest upstream changes (presumably #93119) made this pull request unmergeable. Please resolve the merge conflicts. |
readdir() is preferred over readdir_r() on Linux and many other platforms because it more gracefully supports long file names. Both glibc and musl (and presumably all other Linux libc implementations) guarantee that readdir() is thread-safe as long as a single DIR* is not accessed concurrently, which is enough to make a readdir()-based implementation of ReadDir safe. This implementation is already used for some other OSes including Fuchsia, Redox, and Solaris. See rust-lang#40021 for more details. Fixes rust-lang#86649. Fixes rust-lang#34668.
Bionic also guarantees that readdir() is thread-safe enough.
tavianator
force-pushed
the
linux-readdir-no-r
branch
from
64c21ae
to
3eeb3ca
Compare
|
Rebased! Linux and Android tests still pass |
|
@rustbot label -S-waiting-on-author +S-waiting-on-review |
rustbot
added
S-waiting-on-review
|
@bors r+ rollup |
|
📌 Commit 3eeb3ca has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 26, 2022
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#90247 (Improve Duration::try_from_secs_f32/64 accuracy by directly processing exponent and mantissa) - rust-lang#91861 (Replace iterator-based construction of collections by `Into<T>`) - rust-lang#92098 (add OpenBSD platform-support page) - rust-lang#92134 (Add x86_64-pc-windows-msvc linker-plugin-lto instructions) - rust-lang#92256 (Improve selection errors for `~const` trait bounds) - rust-lang#92778 (fs: Use readdir() instead of readdir_r() on Linux and Android) - rust-lang#93338 (Update minifier crate version to 0.0.42) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This was referenced Dec 12, 2022
bors
pushed a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 18, 2022
Bug rust-lang#50619 was fixed by adding an end_of_stream flag in rust-lang#50630. Unfortunately, that fix only applied to the readdir_r() path. When I switched Linux to use readdir() in rust-lang#92778, I inadvertently reintroduced the bug on that platform. Other platforms that had always used readdir() were presumably never fixed. This patch enables end_of_stream for all platforms, and adds a Linux-specific regression test that should hopefully prevent the bug from being reintroduced again.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 18, 2022
…imulacrum fs: Fix rust-lang#50619 (again) and add a regression test Bug rust-lang#50619 was fixed by adding an end_of_stream flag in rust-lang#50630. Unfortunately, that fix only applied to the readdir_r() path. When I switched Linux to use readdir() in rust-lang#92778, I inadvertently reintroduced the bug on that platform. Other platforms that had always used readdir() were presumably never fixed. This patch enables end_of_stream for all platforms, and adds a Linux-specific regression test that should hopefully prevent the bug from being reintroduced again.
Aaron1011
pushed a commit
to Aaron1011/rust
that referenced
this pull request
Jan 6, 2023
…imulacrum fs: Fix rust-lang#50619 (again) and add a regression test Bug rust-lang#50619 was fixed by adding an end_of_stream flag in rust-lang#50630. Unfortunately, that fix only applied to the readdir_r() path. When I switched Linux to use readdir() in rust-lang#92778, I inadvertently reintroduced the bug on that platform. Other platforms that had always used readdir() were presumably never fixed. This patch enables end_of_stream for all platforms, and adds a Linux-specific regression test that should hopefully prevent the bug from being reintroduced again.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See #40021 for more details. Fixes #86649. Fixes #34668.