Skip to content
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

fs: Use readdir() instead of readdir_r() on Linux and Android #92778

Merged
merged 3 commits into from
Jan 27, 2022
Merged

fs: Use readdir() instead of readdir_r() on Linux and Android #92778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c3e92fe
fs: Implement more ReadDir methods in terms of name_cstr()
tavianator Jan 20, 2022
bc04a4e
fs: Use readdir() instead of readdir_r() on Linux
tavianator Jan 11, 2022
3eeb3ca
fs: Use readdir() instead of readdir_r() on Android
tavianator Jan 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 57 additions & 35 deletions library/std/src/sys/unix/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,20 @@ use libc::c_char;
use libc::dirfd;
#[cfg(any(target_os = "linux", target_os = "emscripten"))]
use libc::fstatat64;
#[cfg(any(
target_os = "android",
target_os = "solaris",
target_os = "fuchsia",
target_os = "redox",
target_os = "illumos"
))]
use libc::readdir as readdir64;
#[cfg(target_os = "linux")]
use libc::readdir64;
#[cfg(any(target_os = "emscripten", target_os = "l4re"))]
use libc::readdir64_r;
#[cfg(not(any(
target_os = "android",
target_os = "linux",
target_os = "emscripten",
target_os = "solaris",
Expand All @@ -60,9 +73,7 @@ use libc::{
lstat as lstat64, off_t as off64_t, open as open64, stat as stat64,
};
#[cfg(any(target_os = "linux", target_os = "emscripten", target_os = "l4re"))]
use libc::{
dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, readdir64_r, stat64,
};
use libc::{dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, stat64};

pub use crate::sys_common::fs::try_exists;

Expand Down Expand Up @@ -202,6 +213,8 @@ struct InnerReadDir {
pub struct ReadDir {
inner: Arc<InnerReadDir>,
#[cfg(not(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
Expand All @@ -218,11 +231,12 @@ unsafe impl Sync for Dir {}
pub struct DirEntry {
entry: dirent64,
dir: Arc<InnerReadDir>,
// We need to store an owned copy of the entry name
// on Solaris and Fuchsia because a) it uses a zero-length
// array to store the name, b) its lifetime between readdir
// calls is not guaranteed.
// We need to store an owned copy of the entry name on platforms that use
// readdir() (not readdir_r()), because a) struct dirent may use a flexible
// array to store the name, b) it lives only until the next readdir() call.
#[cfg(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
Expand Down Expand Up @@ -449,6 +463,8 @@ impl Iterator for ReadDir {
type Item = io::Result<DirEntry>;

#[cfg(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "fuchsia",
target_os = "redox",
Expand All @@ -457,12 +473,13 @@ impl Iterator for ReadDir {
fn next(&mut self) -> Option<io::Result<DirEntry>> {
unsafe {
loop {
// Although readdir_r(3) would be a correct function to use here because
// of the thread safety, on Illumos and Fuchsia the readdir(3C) function
// is safe to use in threaded applications and it is generally preferred
// over the readdir_r(3C) function.
// As of POSIX.1-2017, readdir() is not required to be thread safe; only
// readdir_r() is. However, readdir_r() cannot correctly handle platforms
// with unlimited or variable NAME_MAX. Many modern platforms guarantee
// thread safety for readdir() as long an individual DIR* is not accessed
// concurrently, which is sufficient for Rust.
super::os::set_errno(0);
let entry_ptr = libc::readdir(self.inner.dirp.0);
let entry_ptr = readdir64(self.inner.dirp.0);
if entry_ptr.is_null() {
// null can mean either the end is reached or an error occurred.
// So we had to clear errno beforehand to check for an error now.
Expand All @@ -486,6 +503,8 @@ impl Iterator for ReadDir {
}

#[cfg(not(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "fuchsia",
target_os = "redox",
Expand Down Expand Up @@ -531,17 +550,17 @@ impl Drop for Dir {

impl DirEntry {
pub fn path(&self) -> PathBuf {
self.dir.root.join(OsStr::from_bytes(self.name_bytes()))
self.dir.root.join(self.file_name_os_str())
}

pub fn file_name(&self) -> OsString {
OsStr::from_bytes(self.name_bytes()).to_os_string()
self.file_name_os_str().to_os_string()
}

#[cfg(any(target_os = "linux", target_os = "emscripten", target_os = "android"))]
pub fn metadata(&self) -> io::Result<FileAttr> {
let fd = cvt(unsafe { dirfd(self.dir.dirp.0) })?;
let name = self.entry.d_name.as_ptr();
let name = self.name_cstr().as_ptr();

cfg_has_statx! {
if let Some(ret) = unsafe { try_statx(
Expand Down Expand Up @@ -639,29 +658,21 @@ impl DirEntry {
)
}
}
#[cfg(any(
target_os = "android",
target_os = "linux",
target_os = "emscripten",
target_os = "l4re",
target_os = "haiku",
target_os = "vxworks",
target_os = "espidf"
))]
fn name_bytes(&self) -> &[u8] {
unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()).to_bytes() }
}
#[cfg(any(
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
target_os = "redox"
))]
#[cfg(not(any(
target_os = "macos",
target_os = "ios",
target_os = "netbsd",
target_os = "openbsd",
target_os = "freebsd",
target_os = "dragonfly"
)))]
fn name_bytes(&self) -> &[u8] {
self.name.as_bytes()
self.name_cstr().to_bytes()
}

#[cfg(not(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
Expand All @@ -670,7 +681,14 @@ impl DirEntry {
fn name_cstr(&self) -> &CStr {
unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) }
}
#[cfg(any(target_os = "solaris", target_os = "illumos", target_os = "fuchsia"))]
#[cfg(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
target_os = "redox"
))]
fn name_cstr(&self) -> &CStr {
&self.name
}
Expand Down Expand Up @@ -1076,6 +1094,8 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
Ok(ReadDir {
inner: Arc::new(inner),
#[cfg(not(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
Expand Down Expand Up @@ -1611,6 +1631,8 @@ mod remove_dir_impl {
ReadDir {
inner: Arc::new(InnerReadDir { dirp, root: dummy_root }),
#[cfg(not(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/unix/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub fn errno() -> i32 {
}

/// Sets the platform-specific value of errno
#[cfg(all(not(target_os = "linux"), not(target_os = "dragonfly"), not(target_os = "vxworks")))] // needed for readdir and syscall!
#[cfg(all(not(target_os = "dragonfly"), not(target_os = "vxworks")))] // needed for readdir and syscall!
#[allow(dead_code)] // but not all target cfgs actually end up using it
pub fn set_errno(e: i32) {
unsafe { *errno_location() = e as c_int }
Expand Down