From 0c4ac6b33d9c917c38d4d1fb7e263cb6e7bc049d Mon Sep 17 00:00:00 2001 From: Angie Xiang Date: Thu, 20 Feb 2025 13:38:13 +0000 Subject: [PATCH 1/3] Do not block when handling dir containing a named pipe --- src/_impl.rs | 35 ++++++++++++++++++----------------- src/lib.rs | 9 +++++++++ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/_impl.rs b/src/_impl.rs index 354c278..ab43b33 100644 --- a/src/_impl.rs +++ b/src/_impl.rs @@ -178,29 +178,30 @@ fn scan_and_remove_entry_recursively( opts.read(true) .write(fs_at::OpenOptionsWriteMode::Write) .follow(false); - let child_result = opts.open_dir_at(dirfd, name); - let is_dir = match child_result { + let child_result = opts.open_at(dirfd, name); + let child_file = match child_result { // We expect is_eloop to be the only error + Err(e) if e.raw_os_error() == Some(libc::EISDIR) => { + Some(opts.open_dir_at(dirfd, name)?) + } Err(e) if !I::is_eloop(&e) => return Err(e), - Err(_) => false, + Err(_) => None, Ok(child_file) => { let metadata = child_file.metadata()?; - let is_dir = metadata.is_dir(); - if is_dir { - remove_dir_contents_recursive::(child_file, &dir_debug_root, parallel)?; - #[cfg(feature = "log")] - log::trace!("rmdir: {}", &dir_debug_root); - let opts = fs_at::OpenOptions::default(); - opts.rmdir_at(dirfd, name).map_err(|e| { - #[cfg(feature = "log")] - log::debug!("error removing {}", dir_debug_root); - e - })?; - } - is_dir + metadata.is_dir().then_some(child_file) } }; - if !is_dir { + if let Some(child_file) = child_file { + remove_dir_contents_recursive::(child_file, &dir_debug_root, parallel)?; + #[cfg(feature = "log")] + log::trace!("rmdir: {}", &dir_debug_root); + let opts = fs_at::OpenOptions::default(); + opts.rmdir_at(dirfd, name).map_err(|e| { + #[cfg(feature = "log")] + log::debug!("error removing {}", dir_debug_root); + e + })?; + } else { #[cfg(feature = "log")] log::trace!("unlink: {}", &dir_debug_root); opts.unlink_at(dirfd, name).map_err(|e| { diff --git a/src/lib.rs b/src/lib.rs index b8e04d3..5808008 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -341,10 +341,19 @@ mod tests { let ours = tmp.path().join("t.mkdir"); let file = ours.join("file"); let nested = ours.join("another_dir"); + let fifo = ours.join("fifo"); fs::create_dir(&ours)?; fs::create_dir(nested)?; File::create(&file)?; File::open(&file)?; + #[cfg(unix)] + { + unsafe { + use std::ffi::CString; + let fifo_cstr = CString::new(fifo.to_str().unwrap()).unwrap(); + libc::mkfifo(fifo_cstr.as_ptr(), 0o644); + }; + } Ok(Prep { _tmp: tmp, ours, From 566eedac766ece3ccb12d9741c0cd707b27dc90f Mon Sep 17 00:00:00 2001 From: Angie Xiang Date: Fri, 21 Feb 2025 16:36:19 +0000 Subject: [PATCH 2/3] Fix comments in scan_and_remove_entry_recursively --- src/_impl.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/_impl.rs b/src/_impl.rs index ab43b33..e807e78 100644 --- a/src/_impl.rs +++ b/src/_impl.rs @@ -180,10 +180,11 @@ fn scan_and_remove_entry_recursively( .follow(false); let child_result = opts.open_at(dirfd, name); let child_file = match child_result { - // We expect is_eloop to be the only error + // If we get EISDIR, we open it as a directory Err(e) if e.raw_os_error() == Some(libc::EISDIR) => { Some(opts.open_dir_at(dirfd, name)?) } + // We expect is_eloop to be the only other error Err(e) if !I::is_eloop(&e) => return Err(e), Err(_) => None, Ok(child_file) => { From 102674b3d182557eff2aa0140a0c1903156ae9eb Mon Sep 17 00:00:00 2001 From: Angie Xiang Date: Fri, 21 Feb 2025 16:56:03 +0000 Subject: [PATCH 3/3] Remove unnecessary code The last case would always return None since we already checked whether this is a directory. In case we were handling a file and no error (with the exception of is_eloop) occurred, set the file handle to None. For clarity, the file handle variable has been renamed to child_directory. --- src/_impl.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/_impl.rs b/src/_impl.rs index e807e78..318ea27 100644 --- a/src/_impl.rs +++ b/src/_impl.rs @@ -179,21 +179,19 @@ fn scan_and_remove_entry_recursively( .write(fs_at::OpenOptionsWriteMode::Write) .follow(false); let child_result = opts.open_at(dirfd, name); - let child_file = match child_result { + let child_directory = match child_result { // If we get EISDIR, we open it as a directory Err(e) if e.raw_os_error() == Some(libc::EISDIR) => { Some(opts.open_dir_at(dirfd, name)?) } + // The below options cover files that are not directories // We expect is_eloop to be the only other error Err(e) if !I::is_eloop(&e) => return Err(e), Err(_) => None, - Ok(child_file) => { - let metadata = child_file.metadata()?; - metadata.is_dir().then_some(child_file) - } + Ok(_) => None, }; - if let Some(child_file) = child_file { - remove_dir_contents_recursive::(child_file, &dir_debug_root, parallel)?; + if let Some(child_directory) = child_directory { + remove_dir_contents_recursive::(child_directory, &dir_debug_root, parallel)?; #[cfg(feature = "log")] log::trace!("rmdir: {}", &dir_debug_root); let opts = fs_at::OpenOptions::default();