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

Author: RenTrieu

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<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!(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<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,49 +43,49 @@ 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);
+    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 {
             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);
 }
 
-/// Test `get_cur_dir()` and `set_cur_dir()`
+/// Test `current_dir()` and `set_current_dir()`
 pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
     /* 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);
 
     // 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<Shell>) {
     // 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<Shell>) {
 
     // 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);
 }

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<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 +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::<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 +66,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,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::<CStr16>(name).cast();
-        let value_ptr: *const Char16 = core::ptr::from_ref::<CStr16>(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()) }