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 <phip1611@gmail.com>

uefi-test-runner/src/proto/shell.rs

@@ -100,14 +100,12 @@ pub fn test_current_dir(shell: &ScopedProtocol<Shell>) {
  assert_eq!(cur_fs_str, expected_fs_str);
 }
 
-/// Test ``get_env()`` and ``set_env()``
+/// Test ``get_env()``, ``get_envs()``, and ``set_env()``
 pub fn test_env(shell: &ScopedProtocol<Shell>) {
  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()
@@ -116,27 +114,35 @@ pub fn test_env(shell: &ScopedProtocol<Shell>) {
  *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);
 }
 
 pub fn test() {

uefi/src/proto/shell/mod.rs

@@ -56,64 +56,65 @@ impl Shell {
  unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) }.to_result()
  }
 
- /// 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<env_value>)` - Value of the environment variable
- /// * `Some(Vec<env_names>)` - Vector of environment variable names
- /// * `None` - Environment variable doesn't exist
+ /// * `Some(<env_value>)` - &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<Vec<&'a CStr16>> {
- let mut env_vec = Vec::new();
- match name {
- Some(n) => {
- let name_ptr: *const Char16 = core::ptr::from_ref::<CStr16>(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::<CStr16>(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<env_names>` - 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 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 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;
  }
  }
- Some(env_vec)
+ env_vec
  }
 
  /// Sets the environment variable
@@ -122,12 +123,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::<CStr16>(name).cast();
  let value_ptr: *const Char16 = core::ptr::from_ref::<CStr16>(value).cast();