From 61bd4e9dda5a0435f85e25615286f66724f5f0fd Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 18 Jan 2022 17:42:47 -0800 Subject: [PATCH 1/3] WIP: looping version of unix remove_dir_all --- library/std/src/sys/unix/fs.rs | 99 +++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 32 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 5b2199c2b7fa4..1842499862d0c 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1672,55 +1672,90 @@ mod remove_dir_impl { } } - fn remove_dir_all_recursive(parent_fd: Option, p: &Path) -> io::Result<()> { - let pcstr = cstr(p)?; - - // entry is expected to be a directory, open as such - let fd = openat_nofollow_dironly(parent_fd, &pcstr)?; - - // open the directory passing ownership of the fd - let (dir, fd) = fdreaddir(fd)?; - for child in dir { - let child = child?; - match is_dir(&child) { - Some(true) => { - remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; - } - Some(false) => { - cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) })?; - } - None => match cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) }) { + fn unlink_direntry(ent: &DirEntry, parent_fd: RawFd) -> io::Result { + match is_dir(&ent) { + Some(true) => Ok(false), + Some(false) => { + cvt(unsafe { unlinkat(parent_fd, ent.name_cstr().as_ptr(), 0) })?; + Ok(true) + } + None => { + match cvt(unsafe { unlinkat(parent_fd, ent.name_cstr().as_ptr(), 0) }) { // type unknown - try to unlink Err(err) if err.raw_os_error() == Some(libc::EISDIR) || err.raw_os_error() == Some(libc::EPERM) => { - // if the file is a directory unlink fails with EISDIR on Linux and EPERM everyhwere else - remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; - } - result => { - result?; + // if the file is a directory unlink fails with EISDIR on Linux + // and EPERM everyhwere else + Ok(false) } - }, + result => result.map(|_| true), + } } } + } - // unlink the directory after removing its contents - cvt(unsafe { - unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), pcstr.as_ptr(), libc::AT_REMOVEDIR) - })?; - Ok(()) + fn remove_dir_all_loop(p: &Path) -> io::Result<()> { + use crate::ffi::CString; + + struct State { + dir: ReadDir, + fd: RawFd, + parent_fd: Option, + pcstr: CString, + } + + impl State { + fn new(parent_fd: Option, pcstr: CString) -> io::Result { + // entry is expected to be a directory, open as such + let fd = openat_nofollow_dironly(parent_fd, &pcstr)?; + + // open the directory passing ownership of the fd + let (dir, fd) = fdreaddir(fd)?; + + Ok(Self { dir, fd, parent_fd, pcstr }) + } + } + + let mut parents = Vec::::new(); + let mut current = State::new(None, cstr(p)?)?; + loop { + while let Some(child) = current.dir.next() { + let child = child?; + if !unlink_direntry(&child, current.fd)? { + // Descend into this child directory + let parent = current; + current = State::new(Some(parent.fd), child.name_cstr().into())?; + parents.push(parent); + } + } + + // unlink the directory after removing its contents + cvt(unsafe { + unlinkat( + current.parent_fd.unwrap_or(libc::AT_FDCWD), + current.pcstr.as_ptr(), + libc::AT_REMOVEDIR, + ) + })?; + + match parents.pop() { + Some(parent) => current = parent, + None => return Ok(()), + } + } } pub fn remove_dir_all(p: &Path) -> io::Result<()> { - // We cannot just call remove_dir_all_recursive() here because that would not delete a passed - // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse + // We cannot just call remove_dir_all_loop() here because that would not delete a passed + // symlink. No need to worry about races, because remove_dir_all_loop() does not descend // into symlinks. let attr = lstat(p)?; if attr.file_type().is_symlink() { crate::fs::remove_file(p) } else { - remove_dir_all_recursive(None, p) + remove_dir_all_loop(p) } } } From 7cf62c75902e649fb50f8bb47ad44117065a9ba1 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Sun, 30 Jan 2022 08:13:56 +0100 Subject: [PATCH 2/3] Temporarily remove special-cased Macos x86-64 implementation. --- library/std/src/sys/unix/fs.rs | 118 --------------------------------- 1 file changed, 118 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 1842499862d0c..cb0cad9e0682b 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1474,124 +1474,6 @@ mod remove_dir_impl { pub use crate::sys_common::fs::remove_dir_all; } -// Dynamically choose implementation Macos x86-64: modern for 10.10+, fallback for older versions -#[cfg(all(target_os = "macos", target_arch = "x86_64"))] -mod remove_dir_impl { - use super::{cstr, lstat, Dir, InnerReadDir, ReadDir}; - use crate::ffi::CStr; - use crate::io; - use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd}; - use crate::os::unix::prelude::{OwnedFd, RawFd}; - use crate::path::{Path, PathBuf}; - use crate::sync::Arc; - use crate::sys::weak::weak; - use crate::sys::{cvt, cvt_r}; - use libc::{c_char, c_int, DIR}; - - pub fn openat_nofollow_dironly(parent_fd: Option, p: &CStr) -> io::Result { - weak!(fn openat(c_int, *const c_char, c_int) -> c_int); - let fd = cvt_r(|| unsafe { - openat.get().unwrap()( - parent_fd.unwrap_or(libc::AT_FDCWD), - p.as_ptr(), - libc::O_CLOEXEC | libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_DIRECTORY, - ) - })?; - Ok(unsafe { OwnedFd::from_raw_fd(fd) }) - } - - fn fdreaddir(dir_fd: OwnedFd) -> io::Result<(ReadDir, RawFd)> { - weak!(fn fdopendir(c_int) -> *mut DIR, "fdopendir$INODE64"); - let ptr = unsafe { fdopendir.get().unwrap()(dir_fd.as_raw_fd()) }; - if ptr.is_null() { - return Err(io::Error::last_os_error()); - } - let dirp = Dir(ptr); - // file descriptor is automatically closed by libc::closedir() now, so give up ownership - let new_parent_fd = dir_fd.into_raw_fd(); - // a valid root is not needed because we do not call any functions involving the full path - // of the DirEntrys. - let dummy_root = PathBuf::new(); - Ok(( - ReadDir { - inner: Arc::new(InnerReadDir { dirp, root: dummy_root }), - end_of_stream: false, - }, - new_parent_fd, - )) - } - - fn remove_dir_all_recursive(parent_fd: Option, p: &Path) -> io::Result<()> { - weak!(fn unlinkat(c_int, *const c_char, c_int) -> c_int); - - let pcstr = cstr(p)?; - - // entry is expected to be a directory, open as such - let fd = openat_nofollow_dironly(parent_fd, &pcstr)?; - - // open the directory passing ownership of the fd - let (dir, fd) = fdreaddir(fd)?; - for child in dir { - let child = child?; - match child.entry.d_type { - libc::DT_DIR => { - remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; - } - libc::DT_UNKNOWN => { - match cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) }) - { - // type unknown - try to unlink - Err(err) if err.raw_os_error() == Some(libc::EPERM) => { - // if the file is a directory unlink fails with EPERM - remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; - } - result => { - result?; - } - } - } - _ => { - // not a directory -> unlink - cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) })?; - } - } - } - - // unlink the directory after removing its contents - cvt(unsafe { - unlinkat.get().unwrap()( - parent_fd.unwrap_or(libc::AT_FDCWD), - pcstr.as_ptr(), - libc::AT_REMOVEDIR, - ) - })?; - Ok(()) - } - - fn remove_dir_all_modern(p: &Path) -> io::Result<()> { - // We cannot just call remove_dir_all_recursive() here because that would not delete a passed - // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse - // into symlinks. - let attr = lstat(p)?; - if attr.file_type().is_symlink() { - crate::fs::remove_file(p) - } else { - remove_dir_all_recursive(None, p) - } - } - - pub fn remove_dir_all(p: &Path) -> io::Result<()> { - weak!(fn openat(c_int, *const c_char, c_int) -> c_int); - if openat.get().is_some() { - // openat() is available with macOS 10.10+, just like unlinkat() and fdopendir() - remove_dir_all_modern(p) - } else { - // fall back to classic implementation - crate::sys_common::fs::remove_dir_all(p) - } - } -} - // Modern implementation using openat(), unlinkat() and fdopendir() #[cfg(not(any( all(target_os = "macos", target_arch = "x86_64"), From d2cfba13265a3a62dee390f386dd902c91331833 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Sun, 30 Jan 2022 08:15:41 +0100 Subject: [PATCH 3/3] Readd Macos x86-64 remove_dir_all() special case using dynamic symbols. --- library/std/src/sys/unix/fs.rs | 55 ++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index cb0cad9e0682b..26ae920f500df 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1475,11 +1475,7 @@ mod remove_dir_impl { } // Modern implementation using openat(), unlinkat() and fdopendir() -#[cfg(not(any( - all(target_os = "macos", target_arch = "x86_64"), - target_os = "redox", - target_os = "espidf" -)))] +#[cfg(not(any(target_os = "redox", target_os = "espidf")))] mod remove_dir_impl { use super::{cstr, lstat, Dir, DirEntry, InnerReadDir, ReadDir}; use crate::ffi::CStr; @@ -1489,8 +1485,39 @@ mod remove_dir_impl { use crate::path::{Path, PathBuf}; use crate::sync::Arc; use crate::sys::{cvt, cvt_r}; + + #[cfg(not(all(target_os = "macos", target_arch = "x86_64"),))] use libc::{fdopendir, openat, unlinkat}; + #[cfg(all(target_os = "macos", target_arch = "x86_64"))] + mod macos_weak { + use crate::sys::weak::weak; + use libc::{c_char, c_int, DIR}; + + pub unsafe fn openat(dirfd: c_int, pathname: *const c_char, flags: c_int) -> c_int { + weak!(fn openat(c_int, *const c_char, c_int) -> c_int); + openat.get().unwrap()(dirfd, pathname, flags) + } + + pub unsafe fn fdopendir(fd: c_int) -> *mut DIR { + weak!(fn fdopendir(c_int) -> *mut DIR, "fdopendir$INODE64"); + fdopendir.get().unwrap()(fd) + } + + pub unsafe fn unlinkat(dirfd: c_int, pathname: *const c_char, flags: c_int) -> c_int { + weak!(fn unlinkat(c_int, *const c_char, c_int) -> c_int); + unlinkat.get().unwrap()(dirfd, pathname, flags) + } + + pub fn has_openat() -> bool { + weak!(fn openat(c_int, *const c_char, c_int) -> c_int); + openat.get().is_some() + } + } + + #[cfg(all(target_os = "macos", target_arch = "x86_64"))] + use macos_weak::{fdopendir, openat, unlinkat}; + pub fn openat_nofollow_dironly(parent_fd: Option, p: &CStr) -> io::Result { let fd = cvt_r(|| unsafe { openat( @@ -1629,7 +1656,7 @@ mod remove_dir_impl { } } - pub fn remove_dir_all(p: &Path) -> io::Result<()> { + fn remove_dir_all_modern(p: &Path) -> io::Result<()> { // We cannot just call remove_dir_all_loop() here because that would not delete a passed // symlink. No need to worry about races, because remove_dir_all_loop() does not descend // into symlinks. @@ -1640,4 +1667,20 @@ mod remove_dir_impl { remove_dir_all_loop(p) } } + + #[cfg(not(all(target_os = "macos", target_arch = "x86_64")))] + pub fn remove_dir_all(p: &Path) -> io::Result<()> { + remove_dir_all_modern(p) + } + + #[cfg(all(target_os = "macos", target_arch = "x86_64"))] + pub fn remove_dir_all(p: &Path) -> io::Result<()> { + if macos_weak::has_openat() { + // openat() is available with macOS 10.10+, just like unlinkat() and fdopendir() + remove_dir_all_modern(p) + } else { + // fall back to classic implementation + crate::sys_common::fs::remove_dir_all(p) + } + } }