Directly use readdir on Unix

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::<Result<Vec<DirEntry>>>()?
                 .len()
         );
         let dir_present =
             |children: &Vec<DirEntry>, 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::<Result<Vec<_>>>()?;
-            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::<Result<Vec<_>>>()?;
+        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)

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,89 @@ 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<ptr::NonNull<libc::DIR>>,
 }
 
 impl<'a> ReadDirImpl<'a> {
     pub fn new(dir_file: &'a mut File) -> Result<Self> {
-        // 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()
+            })?,
+        );
+
+        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<DirEntryImpl>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        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))
+                }
+            });
     }
 }