Skip to content

Commit

Permalink
fs: Use readdir() instead of readdir_r() on Linux
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tavianator committed Jan 11, 2022
1 parent 2e2c86e commit 4839b81
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
37 changes: 23 additions & 14 deletions library/std/src/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ use libc::c_char;
use libc::dirfd;
#[cfg(any(target_os = "linux", target_os = "emscripten"))]
use libc::fstatat64;
#[cfg(any(
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 = "linux",
target_os = "emscripten",
Expand All @@ -60,9 +71,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::{remove_dir_all, try_exists};

Expand Down Expand Up @@ -202,6 +211,7 @@ struct InnerReadDir {
pub struct ReadDir {
inner: Arc<InnerReadDir>,
#[cfg(not(any(
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
Expand All @@ -223,12 +233,13 @@ pub struct DirEntry {
// array to store the name, b) its lifetime between readdir
// calls is not guaranteed.
#[cfg(any(
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
target_os = "redox"
))]
name: Box<[u8]>,
name: Box<CStr>,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -449,22 +460,21 @@ impl Iterator for ReadDir {
type Item = io::Result<DirEntry>;

#[cfg(any(
target_os = "linux",
target_os = "solaris",
target_os = "fuchsia",
target_os = "redox",
target_os = "illumos"
))]
fn next(&mut self) -> Option<io::Result<DirEntry>> {
use crate::slice;

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.
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 @@ -475,14 +485,11 @@ impl Iterator for ReadDir {
}

let name = (*entry_ptr).d_name.as_ptr();
let namelen = libc::strlen(name) as usize;

let ret = DirEntry {
entry: *entry_ptr,
name: slice::from_raw_parts(name as *const u8, namelen as usize)
.to_owned()
.into_boxed_slice(),
dir: Arc::clone(&self.inner),
name: CStr::from_ptr(name).into(),
};
if ret.name_bytes() != b"." && ret.name_bytes() != b".." {
return Some(Ok(ret));
Expand All @@ -492,6 +499,7 @@ impl Iterator for ReadDir {
}

#[cfg(not(any(
target_os = "linux",
target_os = "solaris",
target_os = "fuchsia",
target_os = "redox",
Expand Down Expand Up @@ -547,7 +555,7 @@ impl DirEntry {
#[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.as_ptr();

cfg_has_statx! {
if let Some(ret) = unsafe { try_statx(
Expand Down Expand Up @@ -647,7 +655,6 @@ impl DirEntry {
}
#[cfg(any(
target_os = "android",
target_os = "linux",
target_os = "emscripten",
target_os = "l4re",
target_os = "haiku",
Expand All @@ -658,13 +665,14 @@ impl DirEntry {
unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()).to_bytes() }
}
#[cfg(any(
target_os = "linux",
target_os = "solaris",
target_os = "illumos",
target_os = "fuchsia",
target_os = "redox"
))]
fn name_bytes(&self) -> &[u8] {
&*self.name
self.name.to_bytes()
}

pub fn file_name_os_str(&self) -> &OsStr {
Expand Down Expand Up @@ -1068,6 +1076,7 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
Ok(ReadDir {
inner: Arc::new(inner),
#[cfg(not(any(
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
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

0 comments on commit 4839b81

Please sign in to comment.