From c35df376925eb4bbde952158ad81ec625132edf4 Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Sun, 7 Aug 2022 22:43:12 +1200 Subject: [PATCH] Directly use readdir on Unix --- src/lib.rs | 37 ++++++++++++---------- src/unix.rs | 88 ++++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 92 insertions(+), 33 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a300a8e..824d689 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -536,28 +536,33 @@ mod tests { fn readdir() -> Result<()> { let (_tmp, mut parent_dir, _pathname) = setup()?; assert_eq!( - 2, + 2, // . and .. read_dir(&mut parent_dir)? .collect::>>()? .len() ); let dir_present = |children: &Vec, name: &OsStr| children.iter().any(|e| e.name() == name); - { - let mut options = OpenOptions::default(); - options.create_new(true).write(OpenOptionsWriteMode::Write); - options.open_at(&mut parent_dir, "1")?; - options.open_at(&mut parent_dir, "2")?; - options.open_at(&mut options.mkdir_at(&mut parent_dir, "child")?, "3")?; - let children = read_dir(&mut parent_dir)?.collect::>>()?; - assert!(dir_present(&children, OsStr::new("1")), "{:?}", children); - assert!(dir_present(&children, OsStr::new("2")), "{:?}", children); - assert!( - dir_present(&children, OsStr::new("child")), - "{:?}", - children - ); - } + + let mut options = OpenOptions::default(); + options.create_new(true).write(OpenOptionsWriteMode::Write); + options.open_at(&mut parent_dir, "1")?; + options.open_at(&mut parent_dir, "2")?; + options.open_at(&mut options.mkdir_at(&mut parent_dir, "child")?, "3")?; + let children = read_dir(&mut parent_dir)?.collect::>>()?; + assert_eq!( + 5, + children.len(), + "directory contains 5 entries (., .., 1, 2, child)" + ); + assert!(dir_present(&children, OsStr::new("1")), "{:?}", children); + assert!(dir_present(&children, OsStr::new("2")), "{:?}", children); + assert!( + dir_present(&children, OsStr::new("child")), + "{:?}", + children + ); + { let mut child = OpenOptions::default() .read(true) diff --git a/src/unix.rs b/src/unix.rs index 08a0181..db2eb43 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -5,6 +5,7 @@ use std::{ marker::PhantomData, os::unix::prelude::{AsRawFd, FromRawFd, OsStrExt}, path::Path, + ptr, }; // This will probably take a few iterations to get right. The idea: always use @@ -19,10 +20,6 @@ cfg_if::cfg_if! { use cvt::cvt_r; use libc::{c_int, mkdirat, mode_t}; -use nix::{ - dir::{self, Dir}, - errno, -}; use crate::{OpenOptions, OpenOptionsWriteMode}; @@ -187,36 +184,93 @@ impl OpenOptionsExt for OpenOptions { #[derive(Debug)] pub(crate) struct ReadDirImpl<'a> { - iter: dir::OwningIter, + // Since we clone the FD, the original FD is now separate. In theory. + // However for Windows we use the File directly, thus here we need to + // pretend. _phantom: PhantomData<&'a File>, + // Set to None after we closedir it. Perhaps we should we impl Send and Sync + // because the data referenced is owned by libc ? + dir: Option>, } impl<'a> ReadDirImpl<'a> { pub fn new(dir_file: &'a mut File) -> Result { - // clone the FD - Nix takes ownership of the FD, but we're not - // implementing TryInto here. + // closedir closes the FD; make a new one that we can close when done with. let new_fd = cvt_r(|| unsafe { libc::fcntl(dir_file.as_raw_fd(), libc::F_DUPFD_CLOEXEC, 0) })?; + let mut dir = Some( + ptr::NonNull::new(unsafe { libc::fdopendir(new_fd) }).ok_or_else(|| { + let _droppable = unsafe { File::from_raw_fd(new_fd) }; + std::io::Error::last_os_error() + })?, + ); + + // If dir_file has had operations on it - such as open_at - its pointer + // might not be at the start of the dir, and fdopendir is documented + // (e.g. BSD man pages) to not rewind the fd - and our cloned fd + // inherits the pointer. + for d in dir.as_mut() { + unsafe { libc::rewinddir(d.as_mut()) }; + } + Ok(ReadDirImpl { - iter: Dir::from_fd(new_fd)?.into_iter(), _phantom: PhantomData, + dir, }) } + + fn close_dir(&mut self) -> Result<()> { + if let Some(ref mut dir) = self.dir { + let result = unsafe { libc::closedir(dir.as_mut()) }; + // call made, clear state + self.dir = None; + cvt_r(|| result)?; + } + Ok(()) + } +} + +impl Drop for ReadDirImpl<'_> { + fn drop(&mut self) { + // like the stdlib, we eat errors occuring during drop, as there is no + // way to get error handling. + let _ = self.close_dir(); + } } impl Iterator for ReadDirImpl<'_> { type Item = Result; fn next(&mut self) -> Option { - let item = self.iter.next(); - match item { - None => None, - Some(Err(_e)) => Some(Err(std::io::Error::from_raw_os_error(errno::errno()))), - Some(Ok(e)) => { - let name = OsStr::from_bytes(e.file_name().to_bytes()).to_os_string(); - Some(Ok(DirEntryImpl { name })) - } - } + let dir = unsafe { self.dir?.as_mut() }; + // the readdir result is only guaranteed valid within the same thread + // and until other calls are made on the same dir stream. Thus we + // perform the required work inside next, allowing the next call to + // readdir to be managed by the single mutable borrower rule in Rust. + // readdir requires errno set to zero. + nix::Error::clear(); + return ptr::NonNull::new(unsafe { libc::readdir(dir) }) + .map(|e| { + Ok(DirEntryImpl { + name: unsafe { + // Step one: C pointer to CStr - referenced data, length not known. + let c_str = std::ffi::CStr::from_ptr(e.as_ref().d_name.as_ptr()); + // Step two: OsStr: referenced data, length calcu;ated + let os_str = OsStr::from_bytes(c_str.to_bytes()); + // Step three: owned copy + os_str.to_os_string() + }, + }) + }) + .or_else(|| { + // NULL result, an error IFF errno has been set. + let err = std::io::Error::last_os_error(); + if err.raw_os_error() == Some(0) { + None + } else { + Some(Err(err)) + } + }); } }