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

CHANGELOG.md

@@ -5,7 +5,9 @@
 ### Added
 
 - There is a new `fs` module that provides a high-level API for file-system
-  access. The API is close to the `std::fs` module.
+  access. The API is close to the `std::fs` module. The module also provides a
+  `Path` and a `PathBuf` abstraction that is similar to the ones from
+  `std::path`. However, they are adapted for UEFI.
 - Multiple convenience methods for `CString16` and `CStr16`, including:
   - `CStr16::as_slice()`
   - `CStr16::num_chars()`

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/data_types/strs.rs

@@ -175,7 +175,7 @@ impl<'a> TryFrom<&'a CStr> for &'a CStr8 {
     }
 }
 
-/// An UCS-2 null-terminated string.
+/// An UCS-2 null-terminated string slice.
 ///
 /// This type is largely inspired by [`core::ffi::CStr`] with the exception that all characters are
 /// guaranteed to be 16 bit long.
@@ -423,6 +423,12 @@ impl<StrType: AsRef<str> + ?Sized> EqStrUntilNul<StrType> for CStr16 {
     }
 }
 
+impl AsRef<CStr16> for CStr16 {
+    fn as_ref(&self) -> &CStr16 {
+        self
+    }
+}
+
 /// An iterator over the [`Char16`]s in a [`CStr16`].
 #[derive(Debug)]
 pub struct CStr16Iter<'a> {

uefi/src/fs/file_system.rs

@@ -1,24 +1,27 @@
 //! Module for [`FileSystem`].
 
 use super::*;
+use crate::fs::path::{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 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::PathError
     IllegalPath(PathError),
     /// The file or directory was not found in the underlying volume.
     FileNotFound(String),
@@ -40,12 +43,28 @@ pub enum FileSystemError {
     ReadFailure,
     /// Can't parse file content as UTF-8.
     Utf8Error(FromUtf8Error),
-    /// Could not open the given path.
-    OpenError(String),
+    /// Could not open the given path. Carries the path that could not be opened
+    /// and the underlying UEFI error.
+    #[display(fmt = "{path:?}")]
+    OpenError {
+        /// Path that caused the failure.
+        path: String,
+        /// More detailed failure description.
+        error: crate::Error,
+    },
 }
 
 #[cfg(feature = "unstable")]
-impl core::error::Error for FileSystemError {}
+impl core::error::Error for FileSystemError {
+    fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
+        match self {
+            FileSystemError::IllegalPath(e) => Some(e),
+            FileSystemError::Utf8Error(e) => Some(e),
+            FileSystemError::OpenError { path: _path, error } => Some(error),
+            _ => None,
+        }
+    }
+}
 
 impl From<PathError> for FileSystemError {
     fn from(err: PathError) -> Self {
@@ -90,44 +109,45 @@ impl<'a> FileSystem<'a> {
         let path = path.as_ref();
         self.open(path, UefiFileMode::CreateReadWrite, true)
             .map(|_| ())
-            .map_err(|err| {
-                log::debug!("failed to fetch file info: {err:#?}");
-                FileSystemError::OpenError(path.to_string())
-            })
     }
 
     /// 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<()> {
         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);
+        // 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)?;
+            }
+        }
 
-        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.as_str()))
+        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)?;
-        log::debug!("{:#?}", &file.into_type().unwrap());
         let mut file = self.open(path, UefiFileMode::Read, false)?;
         file.get_boxed_info().map_err(|err| {
-            log::debug!("failed to fetch file info: {err:#?}");
-            FileSystemError::OpenError(path.to_string())
+            log::trace!("failed to fetch file info: {err:#?}");
+            FileSystemError::OpenError {
+                path: path.to_cstr16().to_string(),
+                error: err,
+            }
         })
     }
 
@@ -138,11 +158,13 @@ impl<'a> FileSystem<'a> {
         let mut file = self
             .open(path, UefiFileMode::Read, false)?
             .into_regular_file()
-            .ok_or(FileSystemError::NotAFile(path.as_str().to_string()))?;
-        let info = file.get_boxed_info::<FileInfo>().map_err(|e| {
-            log::error!("get info failed: {e:?}");
-            FileSystemError::OpenError(path.as_str().to_string())
-        })?;
+            .ok_or(FileSystemError::NotAFile(path.to_cstr16().to_string()))?;
+        let info = file
+            .get_boxed_info::<FileInfo>()
+            .map_err(|err| FileSystemError::OpenError {
+                path: path.to_cstr16().to_string(),
+                error: err,
+            })?;
 
         let mut vec = vec![0; info.file_size() as usize];
         let read_bytes = file.read(vec.as_mut_slice()).map_err(|e| {
@@ -164,7 +186,7 @@ impl<'a> FileSystem<'a> {
         let dir = self
             .open(path, UefiFileMode::Read, false)?
             .into_directory()
-            .ok_or(FileSystemError::NotADirectory(path.as_str().to_string()))?;
+            .ok_or(FileSystemError::NotADirectory(path.to_cstr16().to_string()))?;
         Ok(UefiDirectoryIter::new(dir))
     }
 
@@ -185,16 +207,18 @@ impl<'a> FileSystem<'a> {
         match file {
             FileType::Dir(dir) => dir.delete().map_err(|e| {
                 log::error!("error removing dir: {e:?}");
-                FileSystemError::CantDeleteDirectory(path.as_str().to_string())
+                FileSystemError::CantDeleteDirectory(path.to_cstr16().to_string())
             }),
-            FileType::Regular(_) => Err(FileSystemError::NotADirectory(path.as_str().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.
@@ -209,9 +233,9 @@ impl<'a> FileSystem<'a> {
         match file {
             FileType::Regular(file) => file.delete().map_err(|e| {
                 log::error!("error removing file: {e:?}");
-                FileSystemError::CantDeleteFile(path.as_str().to_string())
+                FileSystemError::CantDeleteFile(path.to_cstr16().to_string())
             }),
-            FileType::Dir(_) => Err(FileSystemError::NotAFile(path.as_str().to_string())),
+            FileType::Dir(_) => Err(FileSystemError::NotAFile(path.to_cstr16().to_string())),
         }
     }
 
@@ -278,19 +302,24 @@ impl<'a> FileSystem<'a> {
         mode: UefiFileMode,
         is_dir: bool,
     ) -> FileSystemResult<UefiFileHandle> {
-        let path = NormalizedPath::new("\\", path)?;
-        log::debug!("normalized path: {path}");
+        validate_path(path)?;
+        log::trace!("open validated path: {path}");
 
         let attr = if mode == UefiFileMode::CreateReadWrite && is_dir {
             FileAttribute::DIRECTORY
         } else {
             FileAttribute::empty()
         };
 
-        self.open_root()?.open(&path, mode, attr).map_err(|x| {
-            log::trace!("Can't open file {path}: {x:?}");
-            FileSystemError::OpenError(path.to_string())
-        })
+        self.open_root()?
+            .open(path.to_cstr16(), mode, attr)
+            .map_err(|err| {
+                log::trace!("Can't open file {path}: {err:?}");
+                FileSystemError::OpenError {
+                    path: path.to_cstr16().to_string(),
+                    error: err,
+                }
+            })
     }
 }
 

uefi/src/fs/mod.rs

@@ -1,14 +1,14 @@
-//! A high-level file system API for UEFI applications close to the `fs` module
-//! from Rust's standard library.
+//! A high-level file system API for UEFI applications close to the `std::fs`
+//! module from Rust's standard library. The main type by this module is
+//! [`FileSystem`].
 //!
 //! # Difference to typical File System Abstractions
 //! Users perform actions on dedicated volumes: For example, the boot volume,
 //! such as a CD-rom, USB-stick, or any other storage device.
 //!
 //! Unlike in the API of typical UNIX file system abstractions, there is
-//! no virtual file system.
-//!
-//! Unlike Windows, there is no way to access volumes by a dedicated name.
+//! no virtual file system. Unlike in Windows, there is no way to access volumes
+//! by a dedicated name.
 //!
 //! # Paths
 //! All paths are absolute and follow the FAT-like file system conventions for
@@ -17,7 +17,8 @@
 //! directory is always `/`, i.e., the root, of the opened volume.
 //!
 //! Symlinks or hard-links are not supported but only directories and regular
-//! files with plain linear paths to them.
+//! files with plain linear paths to them. For more information, see
+//! [`Path`] and [`PathBuf`].
 //!
 //! # API Hints
 //! There are no `File` and `Path` abstractions similar to those from `std` that
@@ -31,14 +32,11 @@
 
 mod dir_entry_iter;
 mod file_system;
-mod normalized_path;
 mod path;
 mod uefi_types;
 
 pub use file_system::{FileSystem, FileSystemError, FileSystemResult};
-pub use normalized_path::{PathError, SEPARATOR, SEPARATOR_STR};
+pub use path::*;
 
 use dir_entry_iter::*;
-use normalized_path::*;
-use path::*;
 use uefi_types::*;