uefi: Revising function calls, names, and return types to better match standard convention

Author: RenTrieu

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

@@ -3,27 +3,26 @@
 use uefi::boot::ScopedProtocol;
 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<Shell>) {
     /* 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_eq!(status, Status::SUCCESS);
+    assert!(shell.var(test_var).is_none());
+    let status = shell.set_var(test_var, test_val, false);
+    assert!(status.is_ok());
     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 +33,7 @@ pub fn test_env(shell: &ScopedProtocol<Shell>) {
         }
     }
     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,24 +42,24 @@ pub fn test_env(shell: &ScopedProtocol<Shell>) {
     }
     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);
-    assert_eq!(status, Status::SUCCESS);
-    assert!(shell.get_env(test_var).is_none());
+    let status = shell.set_var(test_var, test_val, false);
+    assert!(status.is_ok());
+    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 {
             found_var = true;
         }
     }
     assert!(!found_var);
-    let cur_env_vec = shell.get_envs();
+    let cur_env_vec = shell.vars();
     assert_eq!(cur_env_vec.count(), default_len);
 }
 

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)]
@@ -32,17 +31,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<Self::Item> {
-        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 +55,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::<CStr16>(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 +65,9 @@ impl Shell {
         }
     }
 
-    /// Gets the list of environment variables
-    ///
-    /// # Returns
-    ///
-    /// * `Vec<env_names>` - 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::<Char16>(),
@@ -97,10 +87,10 @@ 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::<CStr16>(name).cast();
-        let value_ptr: *const Char16 = core::ptr::from_ref::<CStr16>(value).cast();
-        unsafe { (self.0.set_env)(name_ptr.cast(), value_ptr.cast(), volatile) }
+    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) }.to_result()
     }
 }