fs/path: finalize implementation + integration test

Now, only remove_dir_all is missing. The reason for this is not technical.
Someone just needs to put a little more effort into it.

Author: phip1611

uefi-test-runner/src/fs/mod.rs

@@ -2,7 +2,8 @@
 
 use alloc::string::{String, ToString};
 use alloc::vec::Vec;
-use uefi::fs::{FileSystem, FileSystemError};
+use uefi::cstr16;
+use uefi::fs::{FileSystem, FileSystemError, PathBuf};
 use uefi::proto::media::fs::SimpleFileSystem;
 use uefi::table::boot::ScopedProtocol;
 
@@ -11,47 +12,50 @@ use uefi::table::boot::ScopedProtocol;
 pub fn test(sfs: ScopedProtocol<SimpleFileSystem>) -> Result<(), FileSystemError> {
     let mut fs = FileSystem::new(sfs);
 
-    fs.create_dir("test_file_system_abs")?;
+    // test create dir
+    fs.create_dir(cstr16!("foo_dir"))?;
 
-    // slash is transparently transformed to backslash
-    fs.write("test_file_system_abs/foo", "hello")?;
-    // absolute or relative paths are supported; ./ is ignored
-    fs.copy("\\test_file_system_abs/foo", "\\test_file_system_abs/./bar")?;
-    let read = fs.read("\\test_file_system_abs\\bar")?;
+    // test write, copy, and read
+    let data_to_write = "hello world";
+    fs.write(cstr16!("foo_dir\\foo"), data_to_write)?;
+    // Here, we additionally check that absolute paths work.
+    fs.copy(cstr16!("\\foo_dir\\foo"), cstr16!("\\foo_dir\\foo_cpy"))?;
+    let read = fs.read(cstr16!("foo_dir\\foo_cpy"))?;
     let read = String::from_utf8(read).expect("Should be valid utf8");
-    assert_eq!(read, "hello");
-
-    assert_eq!(
-        fs.try_exists("test_file_system_abs\\barfoo"),
-        Err(FileSystemError::OpenError(
-            "\\test_file_system_abs\\barfoo".to_string()
-        ))
-    );
-    fs.rename("test_file_system_abs\\bar", "test_file_system_abs\\barfoo")?;
-    assert!(fs.try_exists("test_file_system_abs\\barfoo").is_ok());
-
+    assert_eq!(read.as_str(), data_to_write);
+
+    // test copy from non-existent file
+    let err = fs.copy(cstr16!("not_found"), cstr16!("abc"));
+    assert!(matches!(err, Err(FileSystemError::OpenError { .. })));
+
+    // test rename file + path buf replaces / with \
+    fs.rename(
+        PathBuf::from(cstr16!("/foo_dir/foo_cpy")),
+        cstr16!("foo_dir\\foo_cpy2"),
+    )?;
+    // file should not be available after rename
+    let err = fs.read(cstr16!("foo_dir\\foo_cpy"));
+    assert!(matches!(err, Err(FileSystemError::OpenError { .. })));
+
+    // test read dir on a sub dir
     let entries = fs
-        .read_dir("test_file_system_abs")?
-        .map(|e| {
-            e.expect("Should return boxed file info")
-                .file_name()
-                .to_string()
-        })
+        .read_dir(cstr16!("foo_dir"))?
+        .map(|entry| entry.expect("Should be valid").file_name().to_string())
         .collect::<Vec<_>>();
-    assert_eq!(&[".", "..", "foo", "barfoo"], entries.as_slice());
-
-    fs.create_dir("/deeply_nested_test")?;
-    fs.create_dir("/deeply_nested_test/1")?;
-    fs.create_dir("/deeply_nested_test/1/2")?;
-    fs.create_dir("/deeply_nested_test/1/2/3")?;
-    fs.create_dir("/deeply_nested_test/1/2/3/4")?;
-    fs.create_dir_all("/deeply_nested_test/1/2/3/4/5/6/7")?;
-    fs.try_exists("/deeply_nested_test/1/2/3/4/5/6/7")?;
+    assert_eq!(&[".", "..", "foo", "foo_cpy2"], entries.as_slice());
+
+    // test create dir recursively
+    fs.create_dir_all(cstr16!("foo_dir\\1\\2\\3\\4\\5\\6\\7"))?;
+    fs.create_dir_all(cstr16!("foo_dir\\1\\2\\3\\4\\5\\6\\7\\8"))?;
+    fs.write(
+        cstr16!("foo_dir\\1\\2\\3\\4\\5\\6\\7\\8\\foobar"),
+        data_to_write,
+    )?;
+    let boxinfo = fs.metadata(cstr16!("foo_dir\\1\\2\\3\\4\\5\\6\\7\\8\\foobar"))?;
+    assert_eq!(boxinfo.file_size(), data_to_write.len() as u64);
+
+    // test remove dir all
     // TODO
-    // fs.remove_dir_all("/deeply_nested_test/1/2/3/4/5/6/7")?;
-    fs.remove_dir("/deeply_nested_test/1/2/3/4/5/6/7")?;
-    let exists = matches!(fs.try_exists("/deeply_nested_test/1/2/3/4/5/6/7"), Ok(_));
-    assert!(!exists);
 
     Ok(())
 }

uefi/src/fs/file_system.rs

@@ -1,27 +1,27 @@
 //! Module for [`FileSystem`].
 
 use super::*;
-use crate::fs;
 use crate::fs::path::validation::{validate_path, PathError};
 use crate::proto::media::file::{FileAttribute, FileInfo, FileType};
 use crate::table::boot::ScopedProtocol;
 use alloc::boxed::Box;
 use alloc::string::{FromUtf8Error, String, ToString};
+use alloc::vec;
 use alloc::vec::Vec;
-use alloc::{format, vec};
 use core::fmt;
 use core::fmt::{Debug, Formatter};
 use core::ops::Deref;
 use derive_more::Display;
-use log::info;
-use uefi::Char16;
+use log::debug;
 
 /// All errors that can happen when working with the [`FileSystem`].
 #[derive(Debug, Clone, Display, PartialEq, Eq)]
 pub enum FileSystemError {
     /// Can't open the root directory of the underlying volume.
     CantOpenVolume,
     /// The path is invalid because of the underlying [`PathError`].
+    ///
+    /// [`PathError`]: path::validation::PathError
     IllegalPath(PathError),
     /// The file or directory was not found in the underlying volume.
     FileNotFound(String),
@@ -47,8 +47,10 @@ pub enum FileSystemError {
     /// and the underlying UEFI error.
     #[display(fmt = "{path:?}")]
     OpenError {
+        /// Path that caused the failure.
         path: String,
-        error: crate::Error<>
+        /// More detailed failure description.
+        error: crate::Error,
     },
 }
 
@@ -112,37 +114,39 @@ impl<'a> FileSystem<'a> {
     /// Recursively create a directory and all of its parent components if they
     /// are missing.
     pub fn create_dir_all(&mut self, path: impl AsRef<Path>) -> FileSystemResult<()> {
-        todo!()
-        /*let path = path.as_ref();
-
-        let normalized_path = NormalizedPath::new("\\", path)?;
-        let normalized_path_string = normalized_path.to_string();
-        let normalized_path_pathref = Path::new(&normalized_path_string);
-
-        let iter = || normalized_path_pathref.components(SEPARATOR);
-        iter()
-            .scan(String::new(), |path_acc, component| {
-                if component != Component::RootDir {
-                    *path_acc += SEPARATOR_STR;
-                    *path_acc += format!("{component}").as_str();
-                }
-                info!("path_acc: {path_acc}, component: {component}");
-                Some((component, path_acc.clone()))
-            })
-            .try_for_each(|(_component, full_path)| self.create_dir(full_path.to_cstr16()))*/
+        let path = path.as_ref();
+
+        // Collect all relevant sub paths in a vector.
+        let mut dirs_to_create = vec![path.to_path_buf()];
+        while let Some(parent) = dirs_to_create.last().unwrap().parent() {
+            debug!("parent={parent}");
+            dirs_to_create.push(parent)
+        }
+        // Now reverse, so that we have something like this:
+        // - a
+        // - a\\b
+        // - a\\b\\c
+        dirs_to_create.reverse();
+
+        for parent in dirs_to_create {
+            if self.try_exists(&parent).is_err() {
+                self.create_dir(parent)?;
+            }
+        }
+
+        Ok(())
     }
 
     /// Given a path, query the file system to get information about a file,
     /// directory, etc. Returns [`UefiFileInfo`].
     pub fn metadata(&mut self, path: impl AsRef<Path>) -> FileSystemResult<Box<UefiFileInfo>> {
         let path = path.as_ref();
-        let file = self.open(path, UefiFileMode::Read, false)?;
         let mut file = self.open(path, UefiFileMode::Read, false)?;
         file.get_boxed_info().map_err(|err| {
-            log::debug!("failed to fetch file info: {err:#?}");
+            log::trace!("failed to fetch file info: {err:#?}");
             FileSystemError::OpenError {
                 path: path.to_cstr16().to_string(),
-                error: err
+                error: err,
             }
         })
     }
@@ -155,12 +159,12 @@ impl<'a> FileSystem<'a> {
             .open(path, UefiFileMode::Read, false)?
             .into_regular_file()
             .ok_or(FileSystemError::NotAFile(path.to_cstr16().to_string()))?;
-        let info = file.get_boxed_info::<FileInfo>().map_err(|err| {
-            FileSystemError::OpenError {
+        let info = file
+            .get_boxed_info::<FileInfo>()
+            .map_err(|err| FileSystemError::OpenError {
                 path: path.to_cstr16().to_string(),
-                error: err
-            }
-        })?;
+                error: err,
+            })?;
 
         let mut vec = vec![0; info.file_size() as usize];
         let read_bytes = file.read(vec.as_mut_slice()).map_err(|e| {
@@ -205,14 +209,16 @@ impl<'a> FileSystem<'a> {
                 log::error!("error removing dir: {e:?}");
                 FileSystemError::CantDeleteDirectory(path.to_cstr16().to_string())
             }),
-            FileType::Regular(_) => Err(FileSystemError::NotADirectory(path.to_cstr16().to_string())),
+            FileType::Regular(_) => {
+                Err(FileSystemError::NotADirectory(path.to_cstr16().to_string()))
+            }
         }
     }
 
     /*/// Removes a directory at this path, after removing all its contents. Use
     /// carefully!
-    pub fn remove_dir_all(&mut self, _path: impl AsRef<Path>) -> FileSystemResult<()> {
-        todo!()
+    pub fn remove_dir_all(&mut self, path: impl AsRef<Path>) -> FileSystemResult<()> {
+        let path = path.as_ref();
     }*/
 
     /// Removes a file from the filesystem.
@@ -311,7 +317,7 @@ impl<'a> FileSystem<'a> {
                 log::trace!("Can't open file {path}: {err:?}");
                 FileSystemError::OpenError {
                     path: path.to_cstr16().to_string(),
-                    error: err
+                    error: err,
                 }
             })
     }

uefi/src/fs/mod.rs

@@ -31,8 +31,8 @@
 
 mod dir_entry_iter;
 mod file_system;
-mod uefi_types;
 mod path;
+mod uefi_types;
 
 pub use file_system::{FileSystem, FileSystemError, FileSystemResult};
 pub use path::*;

uefi/src/fs/path/mod.rs

@@ -15,21 +15,22 @@
 
 mod path;
 mod pathbuf;
-pub(super) mod validation;
+pub mod validation;
 
 pub use path::Path;
 pub use pathbuf::PathBuf;
 
-use uefi::{Char16, CStr16};
+use uefi::{CStr16, Char16};
 
 /// The default separator for paths.
 pub const SEPARATOR: char = '\\';
+/// [`SEPARATOR`] but as useful UEFI type.
 pub const SEPARATOR_C16: Char16 = unsafe { Char16::from_u16_unchecked('\\' as u16) };
 
 /// Stringified version of [`SEPARATOR`].
 pub const SEPARATOR_STR: &str = "\\";
+/// [`SEPARATOR_STR`] but as useful UEFI type.
 pub const SEPARATOR_CSTR16: &CStr16 = uefi_macros::cstr16!("\\");
 
 /// Denied characters in a path component.
-pub const CHARACTER_DENY_LIST: [char; 10] =
-    ['\0', '"', '*', '/', ':', '<', '>', '?', '\\', '|'];
+pub const CHARACTER_DENY_LIST: [char; 10] = ['\0', '"', '*', '/', ':', '<', '>', '?', '\\', '|'];