And add Windows readdir support

Cargo.toml

@@ -14,12 +14,12 @@ version = "0.0.1"
 
 [dependencies]
 cfg-if = "1.0.0"
+cvt = "0.1.1"
 
 [dev-dependencies]
 tempfile = "3.3.0"
 
 [target.'cfg(not(windows))'.dependencies]
-cvt = "0.1.1"
 libc = "0.2.121"
 # Saves nontrivial unsafe and platform specific code (Darwin vs other Unixes,
 # MAX_PATH and more) : consider it weak and something we can remove if expedient

README.md

@@ -6,7 +6,7 @@ filesystem code, since otherwise the state of the filesystem path that
 operations are executed against can change silently, leading to TOC-TOU race
 conditions. For Unix these calls are readily available in the libc crate, but
 for Windows some more plumbing is needed. This crate provides a unified
-Rust-y interface to these calls.
+Rust-y and safe interface to these calls.
 
 ## MSRV policy
 

src/lib.rs

@@ -24,7 +24,7 @@ cfg_if::cfg_if! {
     if #[cfg(windows)] {
         mod win;
 
-        use win::OpenOptionsImpl;
+        use win::{OpenOptionsImpl, ReadDirImpl, DirEntryImpl};
     } else {
         mod unix;
 
@@ -184,6 +184,11 @@ impl OpenOptions {
     /// This will honour the options set for creation/append etc, but will only
     /// operate relative to d. To open a file with an absolute path, use the
     /// stdlib fs::OpenOptions.
+    ///
+    /// Note: On Windows this uses low level APIs that do not perform path
+    /// separator translation: if passing a path containing a separator, it must
+    /// be a platform native one. e.g. `foo\\bar` on Windows, vs `foo/bar` on
+    /// most other OS's.
     pub fn open_at<P: AsRef<Path>>(&self, d: &mut File, p: P) -> Result<File> {
         self._impl.open_at(d, OpenOptions::ensure_root(p.as_ref())?)
     }
@@ -232,8 +237,16 @@ impl Iterator for ReadDir<'_> {
 /// The returned type for each entry found by [`read_dir`].
 ///
 /// Each entry represents a single entry inside the directory. Platforms that
-/// provide rich metadata expose this through the methods on DirEntry.
-#[derive(Debug, PartialEq)]
+/// provide rich metadata may in future expose this through methods or extension
+/// traits on DirEntry.
+///
+/// For now however, only the [`name()`] is exposed. This does not imply any
+/// additional IO for most workloads: metadata returned from a directory listing
+/// is inherently racy: presuming that what was a dir, or symlink etc when the
+/// directory was listed, will still be the same when opened is fallible.
+/// Instead, use open_at to open the contents, and then process based on the
+/// type of content found.
+#[derive(Debug)]
 pub struct DirEntry {
     _impl: DirEntryImpl,
 }
@@ -532,12 +545,24 @@ mod tests {
         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 parent_dir, "3")?;
+        options.mkdir_at(&mut parent_dir, "child")?;
+        options.open_at(&mut parent_dir, "child\\3")?;
         let children = read_dir(&mut parent_dir)?.collect::<Result<Vec<_>>>()?;
-        let dir_present = |name: &OsStr| children.iter().find(|e| e.name() == name).is_some();
-        assert!(dir_present(OsStr::new("1")));
-        assert!(dir_present(OsStr::new("2")));
-        assert!(dir_present(OsStr::new("3")));
+        let dir_present =
+            |children: &Vec<DirEntry>, name: &OsStr| children.iter().any(|e| e.name() == name);
+        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)
+            .open_at(&mut parent_dir, "child")?;
+        let children = read_dir(&mut child)?.collect::<Result<Vec<_>>>()?;
+        assert_eq!(3, children.len());
+        assert!(dir_present(&children, OsStr::new("3")), "{:?}", children);
         Ok(())
     }
 }

src/unix.rs

@@ -219,7 +219,7 @@ impl Iterator for ReadDirImpl<'_> {
     }
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug)]
 pub(crate) struct DirEntryImpl {
     e: dir::Entry,
     name: OsString,

src/win.rs

@@ -1,33 +1,40 @@
 mod sugar;
 
 use std::{
-    ffi::c_void,
+    ffi::{c_void, OsStr, OsString},
     fmt,
     fs::File,
     io::Result,
     mem::{size_of, zeroed, MaybeUninit},
-    os::windows::prelude::{AsRawHandle, FromRawHandle, OsStrExt},
+    os::windows::prelude::{AsRawHandle, FromRawHandle, OsStrExt, OsStringExt},
     path::Path,
     ptr::null_mut,
+    slice,
 };
 
 use ntapi::ntioapi::{
-    FILE_CREATE, FILE_CREATED, FILE_DIRECTORY_FILE, FILE_DOES_NOT_EXIST, FILE_EXISTS,
-    FILE_NON_DIRECTORY_FILE, FILE_OPENED, FILE_OPEN_IF, FILE_OVERWRITE_IF, FILE_OVERWRITTEN,
-    FILE_SUPERSEDED, FILE_SYNCHRONOUS_IO_NONALERT,
+    FILE_CREATE, FILE_CREATED, FILE_DIRECTORY_FILE, FILE_DOES_NOT_EXIST, FILE_EXISTS, FILE_OPEN,
+    FILE_OPENED, FILE_OPEN_IF, FILE_OVERWRITE_IF, FILE_OVERWRITTEN, FILE_SUPERSEDED,
+    FILE_SYNCHRONOUS_IO_NONALERT,
 };
 use winapi::{
     ctypes,
     shared::{
         minwindef::ULONG,
         ntdef::{NULL, OBJECT_ATTRIBUTES, OBJ_CASE_INSENSITIVE, PLARGE_INTEGER, PVOID},
-        winerror::ERROR_INVALID_PARAMETER,
+        winerror::{ERROR_INVALID_PARAMETER, ERROR_NO_MORE_FILES},
     },
-    um::winnt::{
-        DELETE, FILE_ATTRIBUTE_NORMAL, FILE_GENERIC_WRITE, FILE_LIST_DIRECTORY, FILE_SHARE_DELETE,
-        FILE_SHARE_READ, FILE_SHARE_WRITE, FILE_TRAVERSE, FILE_WRITE_DATA, GENERIC_READ,
-        GENERIC_WRITE, PSECURITY_QUALITY_OF_SERVICE, SECURITY_CONTEXT_TRACKING_MODE,
-        SECURITY_DESCRIPTOR, SECURITY_QUALITY_OF_SERVICE, SYNCHRONIZE,
+    um::{
+        fileapi::FILE_ID_BOTH_DIR_INFO,
+        minwinbase::{FileIdBothDirectoryInfo, FileIdBothDirectoryRestartInfo},
+        winbase::GetFileInformationByHandleEx,
+        winnt::{
+            DELETE, FILE_ATTRIBUTE_NORMAL, FILE_GENERIC_WRITE, FILE_LIST_DIRECTORY,
+            FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, FILE_TRAVERSE, FILE_WRITE_DATA,
+            GENERIC_READ, GENERIC_WRITE, PSECURITY_QUALITY_OF_SERVICE,
+            SECURITY_CONTEXT_TRACKING_MODE, SECURITY_DESCRIPTOR, SECURITY_QUALITY_OF_SERVICE,
+            SYNCHRONIZE,
+        },
     },
 };
 
@@ -226,8 +233,10 @@ impl OpenOptionsImpl {
         // create options needs to be controlled through OpenOptions too.
         // FILE_SYNCHRONOUS_IO_NONALERT is set by CreateFile with the options
         // Rust itself uses - this lets the OS position tracker work. It also
-        // requires SYNCHRONIZE on the access mode.
-        let create_options = CreateOptions(FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT);
+        // requires SYNCHRONIZE on the access mode. We should permit users to
+        // expect particular types, but until we make that explicit, we need to
+        // open any kind of file when requested # FILE_NON_DIRECTORY_FILE |
+        let create_options = CreateOptions(FILE_SYNCHRONOUS_IO_NONALERT);
 
         self.do_create_file(f, path, desired_access, create_disposition, create_options)
     }
@@ -244,9 +253,11 @@ impl OpenOptionsImpl {
             // its poor ergonomics otherwise.
             Ok(FileDisposition(FILE_CREATE))
         } else {
-            Err(std::io::Error::from_raw_os_error(
-                ERROR_INVALID_PARAMETER as i32,
-            ))
+            // just open the existing file.
+            Ok(FileDisposition(FILE_OPEN))
+            // Err(std::io::Error::from_raw_os_error(
+            //     ERROR_INVALID_PARAMETER as i32,
+            // ))
         }
     }
 
@@ -519,6 +530,116 @@ impl OpenOptionsExt for OpenOptions {
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct ReadDirImpl<'a> {
+    /// FILE_ID_BOTH_DIR_INFO is a variable-length struct, otherwise this would
+    /// be a vec of that. None indicates end of iterator from the OS.
+    buffer: Option<Vec<u8>>,
+    d: &'a mut File,
+    // byte offset in buffer to next entry to yield
+    offset: usize,
+}
+
+impl<'a> ReadDirImpl<'a> {
+    pub fn new(d: &mut File) -> Result<ReadDirImpl> {
+        let mut result = ReadDirImpl {
+            // Start with a page, can always grow it statically or dynamically if
+            // needed.
+            buffer: Some(vec![0_u8; 4096]),
+            d,
+            offset: 0,
+        };
+        // TODO: can this ever fail as FindFirstFile does?
+        result.fill_buffer(FileIdBothDirectoryRestartInfo)?;
+        Ok(result)
+    }
+
+    fn fill_buffer(&mut self, class: ULONG) -> Result<bool> {
+        let buffer = self.buffer.as_mut().ok_or_else(|| {
+            std::io::Error::new(
+                std::io::ErrorKind::Other,
+                "Attempt to fill buffer after end of dir",
+            )
+        })?;
+        // Implement
+        // https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-findnextfilea
+        // without ever doing path resolution... the docs for
+        // GetFileInformationByHandleEx do not mention how to detect end of dir,
+        // but FindNextFile does:
+        //
+        // ```
+        //If the function fails because no more matching files can be found,
+        //the GetLastError function returns ERROR_NO_MORE_FILES.
+        // ```
+        let result = cvt::cvt(unsafe {
+            GetFileInformationByHandleEx(
+                self.d.as_raw_handle(),
+                class,
+                buffer.as_mut_ptr() as *mut c_void,
+                buffer.len() as u32,
+            )
+        });
+        match result {
+            Ok(_) => Ok(false),
+            Err(e) if e.raw_os_error() == Some(ERROR_NO_MORE_FILES as i32) => Ok(true),
+            Err(e) => Err(e),
+        }
+    }
+}
+
+impl Iterator for ReadDirImpl<'_> {
+    type Item = Result<DirEntryImpl>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        // if the buffer is empty, fill it; if the buffer is None, exit early.
+        if self.offset >= self.buffer.as_ref()?.len() {
+            match self.fill_buffer(FileIdBothDirectoryInfo) {
+                Ok(false) => {
+                    self.offset = 0;
+                }
+                Ok(true) => {
+                    self.buffer = None;
+                    return None;
+                }
+                Err(e) => return Some(Err(e)),
+            }
+        }
+        // offset is now valid. Dereference into a struct.
+        let struct_mem = &self.buffer.as_ref()?[self.offset..];
+        let info = unsafe { &*struct_mem.as_ptr().cast::<FILE_ID_BOTH_DIR_INFO>() };
+        self.offset = if info.NextEntryOffset == 0 {
+            self.buffer.as_ref()?.len()
+        } else {
+            info.NextEntryOffset as usize + self.offset
+        };
+
+        let name = OsString::from_wide(unsafe {
+            slice::from_raw_parts(
+                info.FileName.as_ptr(),
+                info.FileNameLength as usize / size_of::<u16>(),
+            )
+        });
+        Some(Ok(DirEntryImpl { name }))
+        //
+        //
+        // Read Attributes, Delete, Synchronize
+        // Disposition:	Open
+        // Options:	Synchronous IO Non-Alert, Open Reparse Point
+        //
+    }
+}
+
+#[derive(Debug)]
+pub(crate) struct DirEntryImpl {
+    name: OsString,
+}
+
+impl DirEntryImpl {
+    pub fn name(&self) -> &OsStr {
+        &self.name
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::{fs::rename, io::Result};