From 92e07bbf62f3065b7b5adcdc6a36daf3b6571bad Mon Sep 17 00:00:00 2001 From: Ren Trieu Date: Sat, 31 May 2025 08:35:44 -0700 Subject: [PATCH 1/6] uefi: Implementing wrappers for EFI Shell env and cur_dir functions This commit implements wrappers for the following EFI Shell Protocol functions: set_env(), get_env(), set_cur_dir(), and get_cur_dir(). --- uefi/src/proto/shell/mod.rs | 131 +++++++++++++++++++++++++++++++++++- 1 file changed, 130 insertions(+), 1 deletion(-) diff --git a/uefi/src/proto/shell/mod.rs b/uefi/src/proto/shell/mod.rs index e7e0dd2f4..4596cc3a5 100644 --- a/uefi/src/proto/shell/mod.rs +++ b/uefi/src/proto/shell/mod.rs @@ -2,12 +2,141 @@ //! EFI Shell Protocol v2.2 -use crate::proto::unsafe_protocol; +#![cfg(feature = "alloc")] + +use alloc::vec::Vec; +use uefi_macros::unsafe_protocol; +use uefi_raw::Status; + +use core::ptr; pub use uefi_raw::protocol::shell::ShellProtocol; +use crate::{CStr16, Char16}; + /// Shell Protocol #[derive(Debug)] #[repr(transparent)] #[unsafe_protocol(uefi_raw::protocol::shell::ShellProtocol::GUID)] pub struct Shell(uefi_raw::protocol::shell::ShellProtocol); + +impl Shell { + /// Gets the environment variable or list of environment variables + /// + /// # Arguments + /// + /// * `name` - The environment variable name of which to retrieve the + /// value + /// If None, will return all defined shell environment + /// variables + /// + /// # Returns + /// + /// * `Some(Vec)` - Value of the environment variable + /// * `Some(Vec)` - Vector of environment variable names + /// * `None` - Environment variable doesn't exist + #[must_use] + pub fn get_env<'a>(&'a self, name: Option<&CStr16>) -> Option> { + let mut env_vec = Vec::new(); + match name { + Some(n) => { + let name_ptr: *const Char16 = core::ptr::from_ref::(n).cast(); + let var_val = unsafe { (self.0.get_env)(name_ptr.cast()) }; + if var_val.is_null() { + return None; + } else { + unsafe { env_vec.push(CStr16::from_ptr(var_val.cast())) }; + } + } + None => { + let cur_env_ptr = unsafe { (self.0.get_env)(ptr::null()) }; + + let mut cur_start = cur_env_ptr; + let mut cur_len = 0; + + let mut i = 0; + let mut null_count = 0; + unsafe { + while null_count <= 1 { + if (*(cur_env_ptr.add(i))) == Char16::from_u16_unchecked(0).into() { + if cur_len > 0 { + env_vec.push(CStr16::from_char16_with_nul_unchecked( + &(*ptr::slice_from_raw_parts(cur_start.cast(), cur_len + 1)), + )); + } + cur_len = 0; + null_count += 1; + } else { + if null_count > 0 { + cur_start = cur_env_ptr.add(i); + } + null_count = 0; + cur_len += 1; + } + i += 1; + } + } + } + } + Some(env_vec) + } + + /// Sets the environment variable + /// + /// # Arguments + /// + /// * `name` - The environment variable for which to set the value + /// * `value` - The new value of the environment variable + /// * `volatile` - Indicates whether or not the variable is volatile or + /// not + /// + /// # Returns + /// + /// * `Status::SUCCESS` The variable was successfully set + pub fn set_env(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Status { + let name_ptr: *const Char16 = core::ptr::from_ref::(name).cast(); + let value_ptr: *const Char16 = core::ptr::from_ref::(value).cast(); + unsafe { (self.0.set_env)(name_ptr.cast(), value_ptr.cast(), volatile) } + } + + /// Returns the current directory on the specified device + /// + /// # Arguments + /// + /// * `file_system_mapping` - The file system mapping for which to get + /// the current directory + /// # Returns + /// + /// * `Some(cwd)` - CStr16 containing the current working directory + /// * `None` - Could not retrieve current directory + #[must_use] + pub fn get_cur_dir<'a>(&'a self, file_system_mapping: Option<&CStr16>) -> Option<&'a CStr16> { + let mapping_ptr: *const Char16 = file_system_mapping.map_or(ptr::null(), |x| (x.as_ptr())); + let cur_dir = unsafe { (self.0.get_cur_dir)(mapping_ptr.cast()) }; + if cur_dir.is_null() { + None + } else { + unsafe { Some(CStr16::from_ptr(cur_dir.cast())) } + } + } + + /// Changes the current directory on the specified device + /// + /// # Arguments + /// + /// * `file_system` - Pointer to the file system's mapped name. + /// * `directory` - Points to the directory on the device specified by + /// `file_system`. + /// # Returns + /// + /// * `Status::SUCCESS` The directory was successfully set + /// + /// # Errors + /// + /// * `Status::EFI_NOT_FOUND` The directory does not exist + pub fn set_cur_dir(&self, file_system: Option<&CStr16>, directory: Option<&CStr16>) -> Status { + let fs_ptr: *const Char16 = file_system.map_or(ptr::null(), |x| (x.as_ptr())); + let dir_ptr: *const Char16 = directory.map_or(ptr::null(), |x| (x.as_ptr())); + unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) } + } +} From 0f30078d13ce498e3041992aabdef445eecb2e43 Mon Sep 17 00:00:00 2001 From: Ren Trieu Date: Sat, 31 May 2025 08:36:19 -0700 Subject: [PATCH 2/6] uefi-test-runner: Added tests for EFI Shell env and cur_dir functions This commit includes tests for the following EFI Shell Protocol functions: get_env(), set_env(), get_cur_dir(), and set_cur_dir(). --- uefi-test-runner/src/proto/shell.rs | 144 +++++++++++++++++++++++++++- 1 file changed, 142 insertions(+), 2 deletions(-) diff --git a/uefi-test-runner/src/proto/shell.rs b/uefi-test-runner/src/proto/shell.rs index 493a22370..e8063b35b 100644 --- a/uefi-test-runner/src/proto/shell.rs +++ b/uefi-test-runner/src/proto/shell.rs @@ -1,13 +1,153 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 -use uefi::boot; +use uefi::boot::ScopedProtocol; use uefi::proto::shell::Shell; +use uefi::{CStr16, boot}; +use uefi_raw::Status; + +/// Test ``get_env()`` and ``set_env()`` +pub fn test_env(shell: &ScopedProtocol) { + let mut test_buf = [0u16; 128]; + + /* Test retrieving list of environment variable names (null input) */ + let cur_env_vec = shell + .get_env(None) + .expect("Could not get environment variable"); + assert_eq!( + *cur_env_vec.first().unwrap(), + CStr16::from_str_with_buf("path", &mut test_buf).unwrap() + ); + assert_eq!( + *cur_env_vec.get(1).unwrap(), + CStr16::from_str_with_buf("nonesting", &mut test_buf).unwrap() + ); + + /* Test setting and getting a specific environment variable */ + let mut test_env_buf = [0u16; 32]; + let test_var = CStr16::from_str_with_buf("test_var", &mut test_env_buf).unwrap(); + let mut test_val_buf = [0u16; 32]; + let test_val = CStr16::from_str_with_buf("test_val", &mut test_val_buf).unwrap(); + assert!(shell.get_env(Some(test_var)).is_none()); + let status = shell.set_env(test_var, test_val, false); + assert_eq!(status, Status::SUCCESS); + let cur_env_str = *shell + .get_env(Some(test_var)) + .expect("Could not get environment variable") + .first() + .unwrap(); + assert_eq!(cur_env_str, test_val); + + /* Test deleting environment variable */ + let test_val = CStr16::from_str_with_buf("", &mut test_val_buf).unwrap(); + let status = shell.set_env(test_var, test_val, false); + assert_eq!(status, Status::SUCCESS); + assert!(shell.get_env(Some(test_var)).is_none()); +} + +/// Test ``get_cur_dir()`` and ``set_cur_dir()`` +pub fn test_cur_dir(shell: &ScopedProtocol) { + let mut test_buf = [0u16; 128]; + + /* Test setting and getting current file system and current directory */ + let mut fs_buf = [0u16; 16]; + let fs_var = CStr16::from_str_with_buf("fs0:", &mut fs_buf).unwrap(); + let mut dir_buf = [0u16; 32]; + let dir_var = CStr16::from_str_with_buf("/", &mut dir_buf).unwrap(); + let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + assert_eq!(status, Status::SUCCESS); + + let cur_fs_str = shell + .get_cur_dir(Some(fs_var)) + .expect("Could not get the current file system mapping"); + let expected_fs_str = CStr16::from_str_with_buf("FS0:\\", &mut test_buf).unwrap(); + assert_eq!(cur_fs_str, expected_fs_str); + + // Changing current file system + let fs_var = CStr16::from_str_with_buf("fs1:", &mut fs_buf).unwrap(); + let dir_var = CStr16::from_str_with_buf("/", &mut dir_buf).unwrap(); + let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + assert_eq!(status, Status::SUCCESS); + + let cur_fs_str = shell + .get_cur_dir(Some(fs_var)) + .expect("Could not get the current file system mapping"); + assert_ne!(cur_fs_str, expected_fs_str); + let expected_fs_str = CStr16::from_str_with_buf("FS1:\\", &mut test_buf).unwrap(); + assert_eq!(cur_fs_str, expected_fs_str); + + // Changing current file system and current directory + let fs_var = CStr16::from_str_with_buf("fs0:", &mut fs_buf).unwrap(); + let dir_var = CStr16::from_str_with_buf("efi/", &mut dir_buf).unwrap(); + let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + assert_eq!(status, Status::SUCCESS); + + let cur_fs_str = shell + .get_cur_dir(Some(fs_var)) + .expect("Could not get the current file system mapping"); + assert_ne!(cur_fs_str, expected_fs_str); + let expected_fs_str = CStr16::from_str_with_buf("FS0:\\efi", &mut test_buf).unwrap(); + assert_eq!(cur_fs_str, expected_fs_str); + + /* Test current working directory cases */ + + // At this point, the current working file system has not been set + // So we expect a NULL output + assert!(shell.get_cur_dir(None).is_none()); + + // Setting the current working file system and current working directory + let dir_var = CStr16::from_str_with_buf("fs0:/", &mut dir_buf).unwrap(); + let status = shell.set_cur_dir(None, Some(dir_var)); + assert_eq!(status, Status::SUCCESS); + let cur_fs_str = shell + .get_cur_dir(Some(fs_var)) + .expect("Could not get the current file system mapping"); + let expected_fs_str = CStr16::from_str_with_buf("FS0:", &mut test_buf).unwrap(); + assert_eq!(cur_fs_str, expected_fs_str); + + let cur_fs_str = shell + .get_cur_dir(None) + .expect("Could not get the current file system mapping"); + assert_eq!(cur_fs_str, expected_fs_str); + + // Changing current working directory + let dir_var = CStr16::from_str_with_buf("/efi", &mut dir_buf).unwrap(); + let status = shell.set_cur_dir(None, Some(dir_var)); + assert_eq!(status, Status::SUCCESS); + let cur_fs_str = shell + .get_cur_dir(Some(fs_var)) + .expect("Could not get the current file system mapping"); + let expected_fs_str = CStr16::from_str_with_buf("FS0:\\efi", &mut test_buf).unwrap(); + assert_eq!(cur_fs_str, expected_fs_str); + let cur_fs_str = shell + .get_cur_dir(None) + .expect("Could not get the current file system mapping"); + assert_eq!(cur_fs_str, expected_fs_str); + + // Changing current directory in a non-current working file system + let fs_var = CStr16::from_str_with_buf("fs0:", &mut fs_buf).unwrap(); + let dir_var = CStr16::from_str_with_buf("efi/tools", &mut dir_buf).unwrap(); + let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + assert_eq!(status, Status::SUCCESS); + let cur_fs_str = shell + .get_cur_dir(None) + .expect("Could not get the current file system mapping"); + assert_ne!(cur_fs_str, expected_fs_str); + + let expected_fs_str = CStr16::from_str_with_buf("FS0:\\efi\\tools", &mut test_buf).unwrap(); + let cur_fs_str = shell + .get_cur_dir(Some(fs_var)) + .expect("Could not get the current file system mapping"); + assert_eq!(cur_fs_str, expected_fs_str); +} pub fn test() { info!("Running shell protocol tests"); let handle = boot::get_handle_for_protocol::().expect("No Shell handles"); - let mut _shell = + let shell = boot::open_protocol_exclusive::(handle).expect("Failed to open Shell protocol"); + + test_env(&shell); + test_cur_dir(&shell); } From 5a27fb8845045bedf1916aa7a4579ee53676e00d Mon Sep 17 00:00:00 2001 From: Ren Trieu Date: Tue, 3 Jun 2025 12:51:05 -0700 Subject: [PATCH 3/6] uefi: Splitting get_env() into two separate functions The UEFI get_env() implementation is used for getting individual environment variable values and also environment variable names. This is not intuitive so this commit splits the function into two dedicated ones: one for accessing values and the other for accessing names. Co-authored-by: Philipp Schuster --- uefi-test-runner/src/proto/shell.rs | 30 ++++---- uefi/src/proto/shell/mod.rs | 107 ++++++++++++++-------------- 2 files changed, 73 insertions(+), 64 deletions(-) diff --git a/uefi-test-runner/src/proto/shell.rs b/uefi-test-runner/src/proto/shell.rs index e8063b35b..34efa8256 100644 --- a/uefi-test-runner/src/proto/shell.rs +++ b/uefi-test-runner/src/proto/shell.rs @@ -5,14 +5,12 @@ use uefi::proto::shell::Shell; use uefi::{CStr16, boot}; use uefi_raw::Status; -/// Test ``get_env()`` and ``set_env()`` +/// Test ``get_env()``, ``get_envs()``, and ``set_env()`` pub fn test_env(shell: &ScopedProtocol) { let mut test_buf = [0u16; 128]; - /* Test retrieving list of environment variable names (null input) */ - let cur_env_vec = shell - .get_env(None) - .expect("Could not get environment variable"); + /* Test retrieving list of environment variable names */ + let cur_env_vec = shell.get_envs(); assert_eq!( *cur_env_vec.first().unwrap(), CStr16::from_str_with_buf("path", &mut test_buf).unwrap() @@ -21,27 +19,35 @@ pub fn test_env(shell: &ScopedProtocol) { *cur_env_vec.get(1).unwrap(), CStr16::from_str_with_buf("nonesting", &mut test_buf).unwrap() ); + let default_len = cur_env_vec.len(); /* Test setting and getting a specific environment variable */ let mut test_env_buf = [0u16; 32]; let test_var = CStr16::from_str_with_buf("test_var", &mut test_env_buf).unwrap(); let mut test_val_buf = [0u16; 32]; let test_val = CStr16::from_str_with_buf("test_val", &mut test_val_buf).unwrap(); - assert!(shell.get_env(Some(test_var)).is_none()); + assert!(shell.get_env(test_var).is_none()); let status = shell.set_env(test_var, test_val, false); assert_eq!(status, Status::SUCCESS); - let cur_env_str = *shell - .get_env(Some(test_var)) - .expect("Could not get environment variable") - .first() - .unwrap(); + let cur_env_str = shell + .get_env(test_var) + .expect("Could not get environment variable"); assert_eq!(cur_env_str, test_val); + assert!(!cur_env_vec.contains(&test_var)); + let cur_env_vec = shell.get_envs(); + assert!(cur_env_vec.contains(&test_var)); + assert_eq!(cur_env_vec.len(), default_len + 1); + /* Test deleting environment variable */ let test_val = CStr16::from_str_with_buf("", &mut test_val_buf).unwrap(); let status = shell.set_env(test_var, test_val, false); assert_eq!(status, Status::SUCCESS); - assert!(shell.get_env(Some(test_var)).is_none()); + assert!(shell.get_env(test_var).is_none()); + + let cur_env_vec = shell.get_envs(); + assert!(!cur_env_vec.contains(&test_var)); + assert_eq!(cur_env_vec.len(), default_len); } /// Test ``get_cur_dir()`` and ``set_cur_dir()`` diff --git a/uefi/src/proto/shell/mod.rs b/uefi/src/proto/shell/mod.rs index 4596cc3a5..e8eb36b9b 100644 --- a/uefi/src/proto/shell/mod.rs +++ b/uefi/src/proto/shell/mod.rs @@ -10,75 +10,76 @@ use uefi_raw::Status; use core::ptr; -pub use uefi_raw::protocol::shell::ShellProtocol; +use uefi_raw::protocol::shell::ShellProtocol; use crate::{CStr16, Char16}; /// Shell Protocol #[derive(Debug)] #[repr(transparent)] -#[unsafe_protocol(uefi_raw::protocol::shell::ShellProtocol::GUID)] -pub struct Shell(uefi_raw::protocol::shell::ShellProtocol); +#[unsafe_protocol(ShellProtocol::GUID)] +pub struct Shell(ShellProtocol); impl Shell { - /// Gets the environment variable or list of environment variables + /// Gets the value of the specified environment variable /// /// # Arguments /// /// * `name` - The environment variable name of which to retrieve the - /// value - /// If None, will return all defined shell environment - /// variables + /// value. /// /// # Returns /// - /// * `Some(Vec)` - Value of the environment variable - /// * `Some(Vec)` - Vector of environment variable names - /// * `None` - Environment variable doesn't exist + /// * `Some()` - &CStr16 containing the value of the + /// environment variable + /// * `None` - If environment variable does not exist #[must_use] - pub fn get_env<'a>(&'a self, name: Option<&CStr16>) -> Option> { - let mut env_vec = Vec::new(); - match name { - Some(n) => { - let name_ptr: *const Char16 = core::ptr::from_ref::(n).cast(); - let var_val = unsafe { (self.0.get_env)(name_ptr.cast()) }; - if var_val.is_null() { - return None; - } else { - unsafe { env_vec.push(CStr16::from_ptr(var_val.cast())) }; - } - } - None => { - let cur_env_ptr = unsafe { (self.0.get_env)(ptr::null()) }; + pub fn get_env(&self, name: &CStr16) -> Option<&CStr16> { + let name_ptr: *const Char16 = core::ptr::from_ref::(name).cast(); + let var_val = unsafe { (self.0.get_env)(name_ptr.cast()) }; + if var_val.is_null() { + None + } else { + unsafe { Some(CStr16::from_ptr(var_val.cast())) } + } + } - let mut cur_start = cur_env_ptr; - let mut cur_len = 0; + /// Gets the list of environment variables + /// + /// # Returns + /// + /// * `Vec` - Vector of environment variable names + #[must_use] + pub fn get_envs(&self) -> Vec<&CStr16> { + let mut env_vec: Vec<&CStr16> = Vec::new(); + let cur_env_ptr = unsafe { (self.0.get_env)(ptr::null()) }; + + let mut cur_start = cur_env_ptr; + let mut cur_len = 0; - let mut i = 0; - let mut null_count = 0; - unsafe { - while null_count <= 1 { - if (*(cur_env_ptr.add(i))) == Char16::from_u16_unchecked(0).into() { - if cur_len > 0 { - env_vec.push(CStr16::from_char16_with_nul_unchecked( - &(*ptr::slice_from_raw_parts(cur_start.cast(), cur_len + 1)), - )); - } - cur_len = 0; - null_count += 1; - } else { - if null_count > 0 { - cur_start = cur_env_ptr.add(i); - } - null_count = 0; - cur_len += 1; - } - i += 1; + let mut i = 0; + let mut null_count = 0; + unsafe { + while null_count <= 1 { + if (*(cur_env_ptr.add(i))) == Char16::from_u16_unchecked(0).into() { + if cur_len > 0 { + env_vec.push(CStr16::from_char16_with_nul( + &(*ptr::slice_from_raw_parts(cur_start.cast(), cur_len + 1)), + ).unwrap()); } + cur_len = 0; + null_count += 1; + } else { + if null_count > 0 { + cur_start = cur_env_ptr.add(i); + } + null_count = 0; + cur_len += 1; } + i += 1; } } - Some(env_vec) + env_vec } /// Sets the environment variable @@ -87,12 +88,12 @@ impl Shell { /// /// * `name` - The environment variable for which to set the value /// * `value` - The new value of the environment variable - /// * `volatile` - Indicates whether or not the variable is volatile or + /// * `volatile` - Indicates whether the variable is volatile or /// not /// /// # Returns /// - /// * `Status::SUCCESS` The variable was successfully set + /// * `Status::SUCCESS` - The variable was successfully set pub fn set_env(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Status { let name_ptr: *const Char16 = core::ptr::from_ref::(name).cast(); let value_ptr: *const Char16 = core::ptr::from_ref::(value).cast(); @@ -105,12 +106,13 @@ impl Shell { /// /// * `file_system_mapping` - The file system mapping for which to get /// the current directory + /// /// # Returns /// /// * `Some(cwd)` - CStr16 containing the current working directory /// * `None` - Could not retrieve current directory #[must_use] - pub fn get_cur_dir<'a>(&'a self, file_system_mapping: Option<&CStr16>) -> Option<&'a CStr16> { + pub fn get_cur_dir(&self, file_system_mapping: Option<&CStr16>) -> Option<&CStr16> { let mapping_ptr: *const Char16 = file_system_mapping.map_or(ptr::null(), |x| (x.as_ptr())); let cur_dir = unsafe { (self.0.get_cur_dir)(mapping_ptr.cast()) }; if cur_dir.is_null() { @@ -127,13 +129,14 @@ impl Shell { /// * `file_system` - Pointer to the file system's mapped name. /// * `directory` - Points to the directory on the device specified by /// `file_system`. + /// /// # Returns /// - /// * `Status::SUCCESS` The directory was successfully set + /// * `Status::SUCCESS` - The directory was successfully set /// /// # Errors /// - /// * `Status::EFI_NOT_FOUND` The directory does not exist + /// * `Status::EFI_NOT_FOUND` - The directory does not exist pub fn set_cur_dir(&self, file_system: Option<&CStr16>, directory: Option<&CStr16>) -> Status { let fs_ptr: *const Char16 = file_system.map_or(ptr::null(), |x| (x.as_ptr())); let dir_ptr: *const Char16 = directory.map_or(ptr::null(), |x| (x.as_ptr())); From a21932f7678710be12b2cb79038d23f6de089453 Mon Sep 17 00:00:00 2001 From: Ren Trieu Date: Tue, 10 Jun 2025 19:02:42 -0700 Subject: [PATCH 4/6] uefi: Changing get_envs() to return an Iterator Co-authored-by: Philipp Schuster --- uefi-test-runner/src/proto/shell.rs | 98 +++++++++++---------- uefi/src/proto/shell/mod.rs | 128 +++++++++++++++++++++------- 2 files changed, 150 insertions(+), 76 deletions(-) diff --git a/uefi-test-runner/src/proto/shell.rs b/uefi-test-runner/src/proto/shell.rs index 34efa8256..23602ba72 100644 --- a/uefi-test-runner/src/proto/shell.rs +++ b/uefi-test-runner/src/proto/shell.rs @@ -2,30 +2,23 @@ use uefi::boot::ScopedProtocol; use uefi::proto::shell::Shell; -use uefi::{CStr16, boot}; +use uefi::{boot, cstr16}; use uefi_raw::Status; -/// Test ``get_env()``, ``get_envs()``, and ``set_env()`` +/// Test `get_env()`, `get_envs()`, and `set_env()` pub fn test_env(shell: &ScopedProtocol) { - let mut test_buf = [0u16; 128]; - /* Test retrieving list of environment variable names */ + let mut cur_env_vec = shell.get_envs(); + assert_eq!(cur_env_vec.next().unwrap(), cstr16!("path"),); + // check pre-defined shell variables; see UEFI Shell spec + assert_eq!(cur_env_vec.next().unwrap(), cstr16!("nonesting"),); let cur_env_vec = shell.get_envs(); - assert_eq!( - *cur_env_vec.first().unwrap(), - CStr16::from_str_with_buf("path", &mut test_buf).unwrap() - ); - assert_eq!( - *cur_env_vec.get(1).unwrap(), - CStr16::from_str_with_buf("nonesting", &mut test_buf).unwrap() - ); - let default_len = cur_env_vec.len(); + let default_len = cur_env_vec.count(); /* Test setting and getting a specific environment variable */ - let mut test_env_buf = [0u16; 32]; - let test_var = CStr16::from_str_with_buf("test_var", &mut test_env_buf).unwrap(); - let mut test_val_buf = [0u16; 32]; - let test_val = CStr16::from_str_with_buf("test_val", &mut test_val_buf).unwrap(); + let cur_env_vec = shell.get_envs(); + let test_var = cstr16!("test_var"); + let test_val = cstr16!("test_val"); assert!(shell.get_env(test_var).is_none()); let status = shell.set_env(test_var, test_val, false); assert_eq!(status, Status::SUCCESS); @@ -34,43 +27,60 @@ pub fn test_env(shell: &ScopedProtocol) { .expect("Could not get environment variable"); assert_eq!(cur_env_str, test_val); - assert!(!cur_env_vec.contains(&test_var)); + let mut found_var = false; + for env_var in cur_env_vec { + if env_var == test_var { + found_var = true; + } + } + assert!(!found_var); + let cur_env_vec = shell.get_envs(); + let mut found_var = false; + for env_var in cur_env_vec { + if env_var == test_var { + found_var = true; + } + } + assert!(found_var); + let cur_env_vec = shell.get_envs(); - assert!(cur_env_vec.contains(&test_var)); - assert_eq!(cur_env_vec.len(), default_len + 1); + assert_eq!(cur_env_vec.count(), default_len + 1); /* Test deleting environment variable */ - let test_val = CStr16::from_str_with_buf("", &mut test_val_buf).unwrap(); + let test_val = cstr16!(""); let status = shell.set_env(test_var, test_val, false); assert_eq!(status, Status::SUCCESS); assert!(shell.get_env(test_var).is_none()); let cur_env_vec = shell.get_envs(); - assert!(!cur_env_vec.contains(&test_var)); - assert_eq!(cur_env_vec.len(), default_len); + let mut found_var = false; + for env_var in cur_env_vec { + if env_var == test_var { + found_var = true; + } + } + assert!(!found_var); + let cur_env_vec = shell.get_envs(); + assert_eq!(cur_env_vec.count(), default_len); } -/// Test ``get_cur_dir()`` and ``set_cur_dir()`` +/// Test `get_cur_dir()` and `set_cur_dir()` pub fn test_cur_dir(shell: &ScopedProtocol) { - let mut test_buf = [0u16; 128]; - /* Test setting and getting current file system and current directory */ - let mut fs_buf = [0u16; 16]; - let fs_var = CStr16::from_str_with_buf("fs0:", &mut fs_buf).unwrap(); - let mut dir_buf = [0u16; 32]; - let dir_var = CStr16::from_str_with_buf("/", &mut dir_buf).unwrap(); + let fs_var = cstr16!("fs0:"); + let dir_var = cstr16!("/"); let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell .get_cur_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); - let expected_fs_str = CStr16::from_str_with_buf("FS0:\\", &mut test_buf).unwrap(); + let expected_fs_str = cstr16!("FS0:\\"); assert_eq!(cur_fs_str, expected_fs_str); // Changing current file system - let fs_var = CStr16::from_str_with_buf("fs1:", &mut fs_buf).unwrap(); - let dir_var = CStr16::from_str_with_buf("/", &mut dir_buf).unwrap(); + let fs_var = cstr16!("fs1:"); + let dir_var = cstr16!("/"); let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); @@ -78,12 +88,12 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { .get_cur_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); assert_ne!(cur_fs_str, expected_fs_str); - let expected_fs_str = CStr16::from_str_with_buf("FS1:\\", &mut test_buf).unwrap(); + let expected_fs_str = cstr16!("FS1:\\"); assert_eq!(cur_fs_str, expected_fs_str); // Changing current file system and current directory - let fs_var = CStr16::from_str_with_buf("fs0:", &mut fs_buf).unwrap(); - let dir_var = CStr16::from_str_with_buf("efi/", &mut dir_buf).unwrap(); + let fs_var = cstr16!("fs0:"); + let dir_var = cstr16!("efi/"); let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); @@ -91,7 +101,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { .get_cur_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); assert_ne!(cur_fs_str, expected_fs_str); - let expected_fs_str = CStr16::from_str_with_buf("FS0:\\efi", &mut test_buf).unwrap(); + let expected_fs_str = cstr16!("FS0:\\efi"); assert_eq!(cur_fs_str, expected_fs_str); /* Test current working directory cases */ @@ -101,13 +111,13 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { assert!(shell.get_cur_dir(None).is_none()); // Setting the current working file system and current working directory - let dir_var = CStr16::from_str_with_buf("fs0:/", &mut dir_buf).unwrap(); + let dir_var = cstr16!("fs0:/"); let status = shell.set_cur_dir(None, Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell .get_cur_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); - let expected_fs_str = CStr16::from_str_with_buf("FS0:", &mut test_buf).unwrap(); + let expected_fs_str = cstr16!("FS0:"); assert_eq!(cur_fs_str, expected_fs_str); let cur_fs_str = shell @@ -116,13 +126,13 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { assert_eq!(cur_fs_str, expected_fs_str); // Changing current working directory - let dir_var = CStr16::from_str_with_buf("/efi", &mut dir_buf).unwrap(); + let dir_var = cstr16!("/efi"); let status = shell.set_cur_dir(None, Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell .get_cur_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); - let expected_fs_str = CStr16::from_str_with_buf("FS0:\\efi", &mut test_buf).unwrap(); + let expected_fs_str = cstr16!("FS0:\\efi"); assert_eq!(cur_fs_str, expected_fs_str); let cur_fs_str = shell .get_cur_dir(None) @@ -130,8 +140,8 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { assert_eq!(cur_fs_str, expected_fs_str); // Changing current directory in a non-current working file system - let fs_var = CStr16::from_str_with_buf("fs0:", &mut fs_buf).unwrap(); - let dir_var = CStr16::from_str_with_buf("efi/tools", &mut dir_buf).unwrap(); + let fs_var = cstr16!("fs0:"); + let dir_var = cstr16!("efi/tools"); let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell @@ -139,7 +149,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { .expect("Could not get the current file system mapping"); assert_ne!(cur_fs_str, expected_fs_str); - let expected_fs_str = CStr16::from_str_with_buf("FS0:\\efi\\tools", &mut test_buf).unwrap(); + let expected_fs_str = cstr16!("FS0:\\efi\\tools"); let cur_fs_str = shell .get_cur_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); diff --git a/uefi/src/proto/shell/mod.rs b/uefi/src/proto/shell/mod.rs index e8eb36b9b..f7e595307 100644 --- a/uefi/src/proto/shell/mod.rs +++ b/uefi/src/proto/shell/mod.rs @@ -2,12 +2,10 @@ //! EFI Shell Protocol v2.2 -#![cfg(feature = "alloc")] - -use alloc::vec::Vec; use uefi_macros::unsafe_protocol; use uefi_raw::Status; +use core::marker::PhantomData; use core::ptr; use uefi_raw::protocol::shell::ShellProtocol; @@ -20,6 +18,35 @@ use crate::{CStr16, Char16}; #[unsafe_protocol(ShellProtocol::GUID)] pub struct Shell(ShellProtocol); +/// Iterator over the names of environmental variables obtained from the Shell protocol. +#[derive(Debug)] +pub struct Vars<'a> { + /// Char16 containing names of environment variables + inner: *const Char16, + /// Placeholder to attach a lifetime to `Vars` + placeholder: PhantomData<&'a CStr16>, +} + +impl<'a> Iterator for Vars<'a> { + type Item = &'a CStr16; + // We iterate a list of NUL terminated CStr16s. + // The list is terminated with a double NUL. + fn next(&mut self) -> Option { + let cur_start = self.inner; + let mut cur_len = 0; + unsafe { + if *(cur_start) == Char16::from_u16_unchecked(0) { + return None; + } + while *(cur_start.add(cur_len)) != Char16::from_u16_unchecked(0) { + cur_len += 1; + } + self.inner = self.inner.add(cur_len + 1); + Some(CStr16::from_ptr(cur_start)) + } + } +} + impl Shell { /// Gets the value of the specified environment variable /// @@ -50,36 +77,12 @@ impl Shell { /// /// * `Vec` - Vector of environment variable names #[must_use] - pub fn get_envs(&self) -> Vec<&CStr16> { - let mut env_vec: Vec<&CStr16> = Vec::new(); - let cur_env_ptr = unsafe { (self.0.get_env)(ptr::null()) }; - - let mut cur_start = cur_env_ptr; - let mut cur_len = 0; - - let mut i = 0; - let mut null_count = 0; - unsafe { - while null_count <= 1 { - if (*(cur_env_ptr.add(i))) == Char16::from_u16_unchecked(0).into() { - if cur_len > 0 { - env_vec.push(CStr16::from_char16_with_nul( - &(*ptr::slice_from_raw_parts(cur_start.cast(), cur_len + 1)), - ).unwrap()); - } - cur_len = 0; - null_count += 1; - } else { - if null_count > 0 { - cur_start = cur_env_ptr.add(i); - } - null_count = 0; - cur_len += 1; - } - i += 1; - } + pub fn get_envs(&self) -> Vars { + let env_ptr = unsafe { (self.0.get_env)(ptr::null()) }; + Vars { + inner: env_ptr.cast::(), + placeholder: PhantomData, } - env_vec } /// Sets the environment variable @@ -143,3 +146,64 @@ impl Shell { unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) } } } + +#[cfg(test)] +mod tests { + use super::*; + use alloc::vec::Vec; + use uefi::cstr16; + + /// Testing Vars struct + #[test] + fn test_vars() { + // Empty Vars + let mut vars_mock = Vec::::new(); + vars_mock.push(0); + vars_mock.push(0); + let mut vars = Vars { + inner: vars_mock.as_ptr().cast(), + placeholder: PhantomData, + }; + assert!(vars.next().is_none()); + + // One environment variable in Vars + let mut vars_mock = Vec::::new(); + vars_mock.push(b'f' as u16); + vars_mock.push(b'o' as u16); + vars_mock.push(b'o' as u16); + vars_mock.push(0); + vars_mock.push(0); + let vars = Vars { + inner: vars_mock.as_ptr().cast(), + placeholder: PhantomData, + }; + assert_eq!(vars.collect::>(), Vec::from([cstr16!("foo")])); + + // Multiple environment variables in Vars + let mut vars_mock = Vec::::new(); + vars_mock.push(b'f' as u16); + vars_mock.push(b'o' as u16); + vars_mock.push(b'o' as u16); + vars_mock.push(b'1' as u16); + vars_mock.push(0); + vars_mock.push(b'b' as u16); + vars_mock.push(b'a' as u16); + vars_mock.push(b'r' as u16); + vars_mock.push(0); + vars_mock.push(b'b' as u16); + vars_mock.push(b'a' as u16); + vars_mock.push(b'z' as u16); + vars_mock.push(b'2' as u16); + vars_mock.push(0); + vars_mock.push(0); + + let vars = Vars { + inner: vars_mock.as_ptr().cast(), + placeholder: PhantomData, + }; + assert_eq!( + vars.collect::>(), + Vec::from([cstr16!("foo1"), cstr16!("bar"), cstr16!("baz2")]) + ); + } +} From 67592781ece12559d0e7c07f9c0dd43783c8d685 Mon Sep 17 00:00:00 2001 From: Ren Trieu Date: Mon, 23 Jun 2025 13:23:57 -0700 Subject: [PATCH 5/6] uefi: Revising function calls and names to better match standard convention --- uefi-test-runner/src/proto/shell.rs | 60 ++++++++++++++--------------- uefi/src/proto/shell/mod.rs | 41 ++++++++------------ 2 files changed, 46 insertions(+), 55 deletions(-) diff --git a/uefi-test-runner/src/proto/shell.rs b/uefi-test-runner/src/proto/shell.rs index 23602ba72..cbb1b925e 100644 --- a/uefi-test-runner/src/proto/shell.rs +++ b/uefi-test-runner/src/proto/shell.rs @@ -5,25 +5,25 @@ use uefi::proto::shell::Shell; use uefi::{boot, cstr16}; use uefi_raw::Status; -/// Test `get_env()`, `get_envs()`, and `set_env()` +/// Test `var()`, `vars()`, and `set_var()` pub fn test_env(shell: &ScopedProtocol) { /* Test retrieving list of environment variable names */ - let mut cur_env_vec = shell.get_envs(); + let mut cur_env_vec = shell.vars(); assert_eq!(cur_env_vec.next().unwrap(), cstr16!("path"),); // check pre-defined shell variables; see UEFI Shell spec assert_eq!(cur_env_vec.next().unwrap(), cstr16!("nonesting"),); - let cur_env_vec = shell.get_envs(); + let cur_env_vec = shell.vars(); let default_len = cur_env_vec.count(); /* Test setting and getting a specific environment variable */ - let cur_env_vec = shell.get_envs(); + let cur_env_vec = shell.vars(); let test_var = cstr16!("test_var"); let test_val = cstr16!("test_val"); - assert!(shell.get_env(test_var).is_none()); - let status = shell.set_env(test_var, test_val, false); + assert!(shell.var(test_var).is_none()); + let status = shell.set_var(test_var, test_val, false); assert_eq!(status, Status::SUCCESS); let cur_env_str = shell - .get_env(test_var) + .var(test_var) .expect("Could not get environment variable"); assert_eq!(cur_env_str, test_val); @@ -34,7 +34,7 @@ pub fn test_env(shell: &ScopedProtocol) { } } assert!(!found_var); - let cur_env_vec = shell.get_envs(); + let cur_env_vec = shell.vars(); let mut found_var = false; for env_var in cur_env_vec { if env_var == test_var { @@ -43,16 +43,16 @@ pub fn test_env(shell: &ScopedProtocol) { } assert!(found_var); - let cur_env_vec = shell.get_envs(); + let cur_env_vec = shell.vars(); assert_eq!(cur_env_vec.count(), default_len + 1); /* Test deleting environment variable */ let test_val = cstr16!(""); - let status = shell.set_env(test_var, test_val, false); + let status = shell.set_var(test_var, test_val, false); assert_eq!(status, Status::SUCCESS); - assert!(shell.get_env(test_var).is_none()); + assert!(shell.var(test_var).is_none()); - let cur_env_vec = shell.get_envs(); + let cur_env_vec = shell.vars(); let mut found_var = false; for env_var in cur_env_vec { if env_var == test_var { @@ -60,20 +60,20 @@ pub fn test_env(shell: &ScopedProtocol) { } } assert!(!found_var); - let cur_env_vec = shell.get_envs(); + let cur_env_vec = shell.vars(); assert_eq!(cur_env_vec.count(), default_len); } -/// Test `get_cur_dir()` and `set_cur_dir()` +/// Test `current_dir()` and `set_current_dir()` pub fn test_cur_dir(shell: &ScopedProtocol) { /* Test setting and getting current file system and current directory */ let fs_var = cstr16!("fs0:"); let dir_var = cstr16!("/"); - let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell - .get_cur_dir(Some(fs_var)) + .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); let expected_fs_str = cstr16!("FS0:\\"); assert_eq!(cur_fs_str, expected_fs_str); @@ -81,11 +81,11 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { // Changing current file system let fs_var = cstr16!("fs1:"); let dir_var = cstr16!("/"); - let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell - .get_cur_dir(Some(fs_var)) + .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); assert_ne!(cur_fs_str, expected_fs_str); let expected_fs_str = cstr16!("FS1:\\"); @@ -94,11 +94,11 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { // Changing current file system and current directory let fs_var = cstr16!("fs0:"); let dir_var = cstr16!("efi/"); - let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell - .get_cur_dir(Some(fs_var)) + .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); assert_ne!(cur_fs_str, expected_fs_str); let expected_fs_str = cstr16!("FS0:\\efi"); @@ -108,50 +108,50 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { // At this point, the current working file system has not been set // So we expect a NULL output - assert!(shell.get_cur_dir(None).is_none()); + assert!(shell.current_dir(None).is_none()); // Setting the current working file system and current working directory let dir_var = cstr16!("fs0:/"); - let status = shell.set_cur_dir(None, Some(dir_var)); + let status = shell.set_current_dir(None, Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell - .get_cur_dir(Some(fs_var)) + .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); let expected_fs_str = cstr16!("FS0:"); assert_eq!(cur_fs_str, expected_fs_str); let cur_fs_str = shell - .get_cur_dir(None) + .current_dir(None) .expect("Could not get the current file system mapping"); assert_eq!(cur_fs_str, expected_fs_str); // Changing current working directory let dir_var = cstr16!("/efi"); - let status = shell.set_cur_dir(None, Some(dir_var)); + let status = shell.set_current_dir(None, Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell - .get_cur_dir(Some(fs_var)) + .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); let expected_fs_str = cstr16!("FS0:\\efi"); assert_eq!(cur_fs_str, expected_fs_str); let cur_fs_str = shell - .get_cur_dir(None) + .current_dir(None) .expect("Could not get the current file system mapping"); assert_eq!(cur_fs_str, expected_fs_str); // Changing current directory in a non-current working file system let fs_var = cstr16!("fs0:"); let dir_var = cstr16!("efi/tools"); - let status = shell.set_cur_dir(Some(fs_var), Some(dir_var)); + let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); assert_eq!(status, Status::SUCCESS); let cur_fs_str = shell - .get_cur_dir(None) + .current_dir(None) .expect("Could not get the current file system mapping"); assert_ne!(cur_fs_str, expected_fs_str); let expected_fs_str = cstr16!("FS0:\\efi\\tools"); let cur_fs_str = shell - .get_cur_dir(Some(fs_var)) + .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); assert_eq!(cur_fs_str, expected_fs_str); } diff --git a/uefi/src/proto/shell/mod.rs b/uefi/src/proto/shell/mod.rs index f7e595307..b6110dd07 100644 --- a/uefi/src/proto/shell/mod.rs +++ b/uefi/src/proto/shell/mod.rs @@ -32,17 +32,12 @@ impl<'a> Iterator for Vars<'a> { // We iterate a list of NUL terminated CStr16s. // The list is terminated with a double NUL. fn next(&mut self) -> Option { - let cur_start = self.inner; - let mut cur_len = 0; - unsafe { - if *(cur_start) == Char16::from_u16_unchecked(0) { - return None; - } - while *(cur_start.add(cur_len)) != Char16::from_u16_unchecked(0) { - cur_len += 1; - } - self.inner = self.inner.add(cur_len + 1); - Some(CStr16::from_ptr(cur_start)) + let s = unsafe { CStr16::from_ptr(self.inner) }; + if s.is_empty() { + None + } else { + self.inner = unsafe { self.inner.add(s.num_chars() + 1) }; + Some(s) } } } @@ -61,8 +56,8 @@ impl Shell { /// environment variable /// * `None` - If environment variable does not exist #[must_use] - pub fn get_env(&self, name: &CStr16) -> Option<&CStr16> { - let name_ptr: *const Char16 = core::ptr::from_ref::(name).cast(); + pub fn var(&self, name: &CStr16) -> Option<&CStr16> { + let name_ptr: *const Char16 = name.as_ptr(); let var_val = unsafe { (self.0.get_env)(name_ptr.cast()) }; if var_val.is_null() { None @@ -71,13 +66,9 @@ impl Shell { } } - /// Gets the list of environment variables - /// - /// # Returns - /// - /// * `Vec` - Vector of environment variable names + /// Gets an iterator over the names of all environment variables #[must_use] - pub fn get_envs(&self) -> Vars { + pub fn vars(&self) -> Vars { let env_ptr = unsafe { (self.0.get_env)(ptr::null()) }; Vars { inner: env_ptr.cast::(), @@ -97,9 +88,9 @@ impl Shell { /// # Returns /// /// * `Status::SUCCESS` - The variable was successfully set - pub fn set_env(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Status { - let name_ptr: *const Char16 = core::ptr::from_ref::(name).cast(); - let value_ptr: *const Char16 = core::ptr::from_ref::(value).cast(); + pub fn set_var(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Status { + let name_ptr: *const Char16 = name.as_ptr(); + let value_ptr: *const Char16 = value.as_ptr(); unsafe { (self.0.set_env)(name_ptr.cast(), value_ptr.cast(), volatile) } } @@ -115,8 +106,8 @@ impl Shell { /// * `Some(cwd)` - CStr16 containing the current working directory /// * `None` - Could not retrieve current directory #[must_use] - pub fn get_cur_dir(&self, file_system_mapping: Option<&CStr16>) -> Option<&CStr16> { - let mapping_ptr: *const Char16 = file_system_mapping.map_or(ptr::null(), |x| (x.as_ptr())); + pub fn current_dir(&self, file_system_mapping: Option<&CStr16>) -> Option<&CStr16> { + let mapping_ptr: *const Char16 = file_system_mapping.map_or(ptr::null(), CStr16::as_ptr); let cur_dir = unsafe { (self.0.get_cur_dir)(mapping_ptr.cast()) }; if cur_dir.is_null() { None @@ -140,7 +131,7 @@ impl Shell { /// # Errors /// /// * `Status::EFI_NOT_FOUND` - The directory does not exist - pub fn set_cur_dir(&self, file_system: Option<&CStr16>, directory: Option<&CStr16>) -> Status { + pub fn set_current_dir(&self, file_system: Option<&CStr16>, directory: Option<&CStr16>) -> Status { let fs_ptr: *const Char16 = file_system.map_or(ptr::null(), |x| (x.as_ptr())); let dir_ptr: *const Char16 = directory.map_or(ptr::null(), |x| (x.as_ptr())); unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) } From 3e9bff85d2175c8afcb9aa427b77dccc27de27de Mon Sep 17 00:00:00 2001 From: Ren Trieu Date: Mon, 23 Jun 2025 13:45:34 -0700 Subject: [PATCH 6/6] uefi: Modifying functions to return Result instead of Status --- uefi-test-runner/src/proto/shell.rs | 17 ++++++++--------- uefi/src/proto/shell/mod.rs | 17 ++++++++++------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/uefi-test-runner/src/proto/shell.rs b/uefi-test-runner/src/proto/shell.rs index cbb1b925e..46843bfea 100644 --- a/uefi-test-runner/src/proto/shell.rs +++ b/uefi-test-runner/src/proto/shell.rs @@ -3,7 +3,6 @@ use uefi::boot::ScopedProtocol; use uefi::proto::shell::Shell; use uefi::{boot, cstr16}; -use uefi_raw::Status; /// Test `var()`, `vars()`, and `set_var()` pub fn test_env(shell: &ScopedProtocol) { @@ -21,7 +20,7 @@ pub fn test_env(shell: &ScopedProtocol) { let test_val = cstr16!("test_val"); assert!(shell.var(test_var).is_none()); let status = shell.set_var(test_var, test_val, false); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); let cur_env_str = shell .var(test_var) .expect("Could not get environment variable"); @@ -49,7 +48,7 @@ pub fn test_env(shell: &ScopedProtocol) { /* Test deleting environment variable */ let test_val = cstr16!(""); let status = shell.set_var(test_var, test_val, false); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); assert!(shell.var(test_var).is_none()); let cur_env_vec = shell.vars(); @@ -70,7 +69,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { let fs_var = cstr16!("fs0:"); let dir_var = cstr16!("/"); let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); let cur_fs_str = shell .current_dir(Some(fs_var)) @@ -82,7 +81,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { let fs_var = cstr16!("fs1:"); let dir_var = cstr16!("/"); let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); let cur_fs_str = shell .current_dir(Some(fs_var)) @@ -95,7 +94,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { let fs_var = cstr16!("fs0:"); let dir_var = cstr16!("efi/"); let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); let cur_fs_str = shell .current_dir(Some(fs_var)) @@ -113,7 +112,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { // Setting the current working file system and current working directory let dir_var = cstr16!("fs0:/"); let status = shell.set_current_dir(None, Some(dir_var)); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); let cur_fs_str = shell .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); @@ -128,7 +127,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { // Changing current working directory let dir_var = cstr16!("/efi"); let status = shell.set_current_dir(None, Some(dir_var)); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); let cur_fs_str = shell .current_dir(Some(fs_var)) .expect("Could not get the current file system mapping"); @@ -143,7 +142,7 @@ pub fn test_cur_dir(shell: &ScopedProtocol) { let fs_var = cstr16!("fs0:"); let dir_var = cstr16!("efi/tools"); let status = shell.set_current_dir(Some(fs_var), Some(dir_var)); - assert_eq!(status, Status::SUCCESS); + assert!(status.is_ok()); let cur_fs_str = shell .current_dir(None) .expect("Could not get the current file system mapping"); diff --git a/uefi/src/proto/shell/mod.rs b/uefi/src/proto/shell/mod.rs index b6110dd07..181653d2d 100644 --- a/uefi/src/proto/shell/mod.rs +++ b/uefi/src/proto/shell/mod.rs @@ -3,14 +3,13 @@ //! EFI Shell Protocol v2.2 use uefi_macros::unsafe_protocol; -use uefi_raw::Status; use core::marker::PhantomData; use core::ptr; use uefi_raw::protocol::shell::ShellProtocol; -use crate::{CStr16, Char16}; +use crate::{CStr16, Char16, Result, StatusExt}; /// Shell Protocol #[derive(Debug)] @@ -68,7 +67,7 @@ impl Shell { /// Gets an iterator over the names of all environment variables #[must_use] - pub fn vars(&self) -> Vars { + pub fn vars(&self) -> Vars<'_> { let env_ptr = unsafe { (self.0.get_env)(ptr::null()) }; Vars { inner: env_ptr.cast::(), @@ -88,10 +87,10 @@ impl Shell { /// # Returns /// /// * `Status::SUCCESS` - The variable was successfully set - pub fn set_var(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Status { + pub fn set_var(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Result { let name_ptr: *const Char16 = name.as_ptr(); let value_ptr: *const Char16 = value.as_ptr(); - unsafe { (self.0.set_env)(name_ptr.cast(), value_ptr.cast(), volatile) } + unsafe { (self.0.set_env)(name_ptr.cast(), value_ptr.cast(), volatile) }.to_result() } /// Returns the current directory on the specified device @@ -131,10 +130,14 @@ impl Shell { /// # Errors /// /// * `Status::EFI_NOT_FOUND` - The directory does not exist - pub fn set_current_dir(&self, file_system: Option<&CStr16>, directory: Option<&CStr16>) -> Status { + pub fn set_current_dir( + &self, + file_system: Option<&CStr16>, + directory: Option<&CStr16>, + ) -> Result { let fs_ptr: *const Char16 = file_system.map_or(ptr::null(), |x| (x.as_ptr())); let dir_ptr: *const Char16 = directory.map_or(ptr::null(), |x| (x.as_ptr())); - unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) } + unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) }.to_result() } }