From c3e92fec948f3875d1fa66d37c8e2599e6140215 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Thu, 20 Jan 2022 16:54:45 -0500 Subject: [PATCH 1/3] fs: Implement more ReadDir methods in terms of name_cstr() --- library/std/src/sys/unix/fs.rs | 41 +++++++++++++++------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index f8deda93fe2a7..14e8f06b3036e 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -531,17 +531,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 { 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( @@ -639,26 +639,16 @@ 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( @@ -670,7 +660,12 @@ 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 = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox" + ))] fn name_cstr(&self) -> &CStr { &self.name } From bc04a4eac47dcdc9feb3061dcc683e9d420227ab Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Tue, 11 Jan 2022 14:09:52 -0500 Subject: [PATCH 2/3] 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 #40021 for more details. Fixes #86649. Fixes #34668. --- library/std/src/sys/unix/fs.rs | 41 ++++++++++++++++++++++++---------- library/std/src/sys/unix/os.rs | 2 +- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 14e8f06b3036e..d1bd1f6d0ebd2 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -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", @@ -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::try_exists; @@ -202,6 +211,7 @@ struct InnerReadDir { pub struct ReadDir { inner: Arc, #[cfg(not(any( + target_os = "linux", target_os = "solaris", target_os = "illumos", target_os = "fuchsia", @@ -218,11 +228,11 @@ unsafe impl Sync for Dir {} pub struct DirEntry { entry: dirent64, dir: Arc, - // 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 = "linux", target_os = "solaris", target_os = "illumos", target_os = "fuchsia", @@ -449,6 +459,7 @@ impl Iterator for ReadDir { type Item = io::Result; #[cfg(any( + target_os = "linux", target_os = "solaris", target_os = "fuchsia", target_os = "redox", @@ -457,12 +468,13 @@ impl Iterator for ReadDir { fn next(&mut self) -> Option> { 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. @@ -486,6 +498,7 @@ impl Iterator for ReadDir { } #[cfg(not(any( + target_os = "linux", target_os = "solaris", target_os = "fuchsia", target_os = "redox", @@ -652,6 +665,7 @@ impl DirEntry { } #[cfg(not(any( + target_os = "linux", target_os = "solaris", target_os = "illumos", target_os = "fuchsia", @@ -661,6 +675,7 @@ impl DirEntry { unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) } } #[cfg(any( + target_os = "linux", target_os = "solaris", target_os = "illumos", target_os = "fuchsia", @@ -1071,6 +1086,7 @@ pub fn readdir(p: &Path) -> io::Result { Ok(ReadDir { inner: Arc::new(inner), #[cfg(not(any( + target_os = "linux", target_os = "solaris", target_os = "illumos", target_os = "fuchsia", @@ -1606,6 +1622,7 @@ mod remove_dir_impl { ReadDir { inner: Arc::new(InnerReadDir { dirp, root: dummy_root }), #[cfg(not(any( + target_os = "linux", target_os = "solaris", target_os = "illumos", target_os = "fuchsia", diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs index 8a028d99306db..7466c77356c7c 100644 --- a/library/std/src/sys/unix/os.rs +++ b/library/std/src/sys/unix/os.rs @@ -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 } From 3eeb3ca40732ac56a9c5a9e4d9dc961644409db2 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Fri, 14 Jan 2022 14:19:09 -0500 Subject: [PATCH 3/3] fs: Use readdir() instead of readdir_r() on Android Bionic also guarantees that readdir() is thread-safe enough. --- library/std/src/sys/unix/fs.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index d1bd1f6d0ebd2..1617188ca60fa 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -35,6 +35,7 @@ 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", @@ -46,6 +47,7 @@ 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", @@ -211,6 +213,7 @@ struct InnerReadDir { pub struct ReadDir { inner: Arc, #[cfg(not(any( + target_os = "android", target_os = "linux", target_os = "solaris", target_os = "illumos", @@ -232,6 +235,7 @@ pub struct DirEntry { // 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", @@ -459,6 +463,7 @@ impl Iterator for ReadDir { type Item = io::Result; #[cfg(any( + target_os = "android", target_os = "linux", target_os = "solaris", target_os = "fuchsia", @@ -498,6 +503,7 @@ impl Iterator for ReadDir { } #[cfg(not(any( + target_os = "android", target_os = "linux", target_os = "solaris", target_os = "fuchsia", @@ -665,6 +671,7 @@ impl DirEntry { } #[cfg(not(any( + target_os = "android", target_os = "linux", target_os = "solaris", target_os = "illumos", @@ -675,6 +682,7 @@ impl DirEntry { unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) } } #[cfg(any( + target_os = "android", target_os = "linux", target_os = "solaris", target_os = "illumos", @@ -1086,6 +1094,7 @@ pub fn readdir(p: &Path) -> io::Result { Ok(ReadDir { inner: Arc::new(inner), #[cfg(not(any( + target_os = "android", target_os = "linux", target_os = "solaris", target_os = "illumos", @@ -1622,6 +1631,7 @@ 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",