From c84b8f752472ddc7c1a418ba385456e96820ed18 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 27 Aug 2022 14:47:06 +0200 Subject: [PATCH 1/3] "is_regular_file" for FileHandle via File trait. --- CHANGELOG.md | 2 ++ src/proto/media/file/dir.rs | 8 +++++++ src/proto/media/file/mod.rs | 42 ++++++++++++++++++++++++++------- src/proto/media/file/regular.rs | 8 +++++++ 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c15f0eba2..a82f85337 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ ### Fixed - Corrected the name of `BlockIOMedia::is_media_preset` to `is_media_present`. +- The `File` trait now knows the methods `is_regular_file` and `is_directory`. + Developers profit from this on the struct `FileHandle`, for example. ### Changed diff --git a/src/proto/media/file/dir.rs b/src/proto/media/file/dir.rs index eb557da35..4f2a79d27 100644 --- a/src/proto/media/file/dir.rs +++ b/src/proto/media/file/dir.rs @@ -67,4 +67,12 @@ impl File for Directory { fn handle(&mut self) -> &mut FileHandle { self.0.handle() } + + fn is_regular_file(&self) -> Result { + Ok(false) + } + + fn is_directory(&self) -> Result { + Ok(true) + } } diff --git a/src/proto/media/file/mod.rs b/src/proto/media/file/mod.rs index 629ea72e0..8150ce87c 100644 --- a/src/proto/media/file/mod.rs +++ b/src/proto/media/file/mod.rs @@ -214,6 +214,18 @@ pub trait File: Sized { // global allocator. unsafe { Ok(Box::from_raw(info)) } } + + /// Returns if the underlying file is a regular file. + /// The result is an error if the underlying file was already closed or deleted. + /// + /// UEFI file system protocol only knows "regular files" and "directories". + fn is_regular_file(&self) -> Result; + + /// Returns if the underlying file is a directory. + /// The result is an error if the underlying file was already closed or deleted. + /// + /// UEFI file system protocol only knows "regular files" and "directories". + fn is_directory(&self) -> Result; } // Internal File helper methods to access the funciton pointer table. @@ -241,16 +253,14 @@ impl FileHandle { } /// Converts `File` into a more specific subtype based on if it is a - /// directory or not. It does this via a call to `get_position`. - pub fn into_type(mut self) -> Result { + /// directory or not. Wrapper around [Self::is_regular_file]. + pub fn into_type(self) -> Result { use FileType::*; - // get_position fails with EFI_UNSUPPORTED on directories - let mut pos = 0; - match (self.imp().get_position)(self.imp(), &mut pos) { - Status::SUCCESS => unsafe { Ok(Regular(RegularFile::new(self))) }, - Status::UNSUPPORTED => unsafe { Ok(Dir(Directory::new(self))) }, - s => Err(s.into()), + if self.is_regular_file()? { + unsafe { Ok(Regular(RegularFile::new(self))) } + } else { + unsafe { Ok(Dir(Directory::new(self))) } } } @@ -280,6 +290,22 @@ impl File for FileHandle { fn handle(&mut self) -> &mut FileHandle { self } + + fn is_regular_file(&self) -> Result { + let this = unsafe { self.0.as_mut().unwrap() }; + + // get_position fails with EFI_UNSUPPORTED on directories + let mut pos = 0; + match (this.get_position)(this, &mut pos) { + Status::SUCCESS => Ok(true), + Status::UNSUPPORTED => Ok(false), + s => Err(s.into()), + } + } + + fn is_directory(&self) -> Result { + self.is_regular_file().map(|b| !b) + } } impl Drop for FileHandle { diff --git a/src/proto/media/file/regular.rs b/src/proto/media/file/regular.rs index af114b4a5..819612ebf 100644 --- a/src/proto/media/file/regular.rs +++ b/src/proto/media/file/regular.rs @@ -104,4 +104,12 @@ impl File for RegularFile { fn handle(&mut self) -> &mut FileHandle { &mut self.0 } + + fn is_regular_file(&self) -> Result { + Ok(true) + } + + fn is_directory(&self) -> Result { + Ok(false) + } } From a8568707ac1aa0e446965b55cac7f91cdc5b72a7 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 27 Aug 2022 14:38:43 +0200 Subject: [PATCH 2/3] error: derive PartialEq This is necessary as preparation for the integration test. --- src/result/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/result/error.rs b/src/result/error.rs index 8c4bde5a0..e04be70ac 100644 --- a/src/result/error.rs +++ b/src/result/error.rs @@ -3,7 +3,7 @@ use core::fmt::Debug; /// Errors emitted from UEFI entry point must propagate erronerous UEFI statuses, /// and may optionally propagate additional entry point-specific data. -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq)] pub struct Error { status: Status, data: Data, From 09b01a9ed6ebc561416baf7befd429c9b4ab480c Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Tue, 6 Sep 2022 17:57:10 +0200 Subject: [PATCH 3/3] integration test for is_file() and is_directory() --- src/proto/media/file/mod.rs | 15 ++++--- .../src/proto/media/known_disk.rs | 45 ++++++++++++------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/proto/media/file/mod.rs b/src/proto/media/file/mod.rs index 8150ce87c..37dd1e9b3 100644 --- a/src/proto/media/file/mod.rs +++ b/src/proto/media/file/mod.rs @@ -257,11 +257,13 @@ impl FileHandle { pub fn into_type(self) -> Result { use FileType::*; - if self.is_regular_file()? { - unsafe { Ok(Regular(RegularFile::new(self))) } - } else { - unsafe { Ok(Dir(Directory::new(self))) } - } + self.is_regular_file().map(|is_file| { + if is_file { + unsafe { Regular(RegularFile::new(self)) } + } else { + unsafe { Dir(Directory::new(self)) } + } + }) } /// If the handle represents a directory, convert it into a @@ -294,7 +296,8 @@ impl File for FileHandle { fn is_regular_file(&self) -> Result { let this = unsafe { self.0.as_mut().unwrap() }; - // get_position fails with EFI_UNSUPPORTED on directories + // - get_position fails with EFI_UNSUPPORTED on directories + // - result is an error if the underlying file was already closed or deleted. let mut pos = 0; match (this.get_position)(this, &mut pos) { Status::SUCCESS => Ok(true), diff --git a/uefi-test-runner/src/proto/media/known_disk.rs b/uefi-test-runner/src/proto/media/known_disk.rs index 72926755c..717859811 100644 --- a/uefi-test-runner/src/proto/media/known_disk.rs +++ b/uefi-test-runner/src/proto/media/known_disk.rs @@ -15,11 +15,13 @@ fn test_existing_dir(directory: &mut Directory) { info!("Testing existing directory"); let input_dir_path = cstr16!("test_dir"); - let mut dir = directory + let dir = directory .open(input_dir_path, FileMode::Read, FileAttribute::empty()) - .expect("failed to open directory") - .into_directory() - .expect("not a directory"); + .expect("failed to open directory"); + + assert!(dir.is_directory().unwrap()); + + let mut dir = dir.into_directory().expect("not a directory"); // Collect and validate the directory entries. let mut entry_names = vec![]; @@ -129,15 +131,17 @@ fn test_create_file(directory: &mut Directory) { info!("Testing file creation"); // Create a new file. - let mut file = directory + let file = directory .open( cstr16!("new_test_file.txt"), FileMode::CreateReadWrite, FileAttribute::empty(), ) - .expect("failed to create file") - .into_regular_file() - .expect("not a regular file"); + .expect("failed to create file"); + + assert!(file.is_regular_file().unwrap()); + + let mut file = file.into_regular_file().expect("not a regular file"); file.write(b"test output data").unwrap(); } @@ -261,10 +265,21 @@ pub fn test_known_disk(bt: &BootServices) { let mut sfs = bt .open_protocol_exclusive::(handle) .expect("Failed to get simple file system"); - let mut directory = sfs.open_volume().unwrap(); + let mut root_directory = sfs.open_volume().unwrap(); + + // test is_directory() and is_regular_file() from the File trait which is the + // base for into_type() used later in the test. + { + // because File is "Sized", we cannot cast it to &dyn + fn test_is_directory(file: &impl File) { + assert_eq!(Ok(true), file.is_directory()); + assert_eq!(Ok(false), file.is_regular_file()); + } + test_is_directory(&root_directory); + } let mut fs_info_buf = vec![0; 128]; - let fs_info = directory + let fs_info = root_directory .get_info::(&mut fs_info_buf) .unwrap(); @@ -281,13 +296,13 @@ pub fn test_known_disk(bt: &BootServices) { assert_eq!(fs_info.block_size(), 512); // Check that `get_boxed_info` returns the same info. - let boxed_fs_info = directory.get_boxed_info::().unwrap(); + let boxed_fs_info = root_directory.get_boxed_info::().unwrap(); assert_eq!(*fs_info, *boxed_fs_info); - test_existing_dir(&mut directory); - test_delete_warning(&mut directory); - test_existing_file(&mut directory); - test_create_file(&mut directory); + test_existing_dir(&mut root_directory); + test_delete_warning(&mut root_directory); + test_existing_file(&mut root_directory); + test_create_file(&mut root_directory); } test_raw_disk_io(handle, bt);