uefi: Revise comments to be more descriptive and use macros for CStr16

Co-authored-by: Philipp Schuster <[email protected]>

Author: RenTrieu

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

@@ -2,32 +2,22 @@
 
 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<Shell>) {
-    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::from_str_with_buf("path", &mut test_buf).unwrap()
-    );
-    assert_eq!(
-        cur_env_vec.next().unwrap(),
-        CStr16::from_str_with_buf("nonesting", &mut test_buf).unwrap()
-    );
+    assert_eq!(cur_env_vec.next().unwrap(), cstr16!("path"),);
+    assert_eq!(cur_env_vec.next().unwrap(), cstr16!("nonesting"),);
     let cur_env_vec = shell.get_envs();
     let default_len = cur_env_vec.count();
 
     /* Test setting and getting a specific environment variable */
     let cur_env_vec = shell.get_envs();
-    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 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);
@@ -56,7 +46,7 @@ pub fn test_env(shell: &ScopedProtocol<Shell>) {
     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());
@@ -73,48 +63,44 @@ pub fn test_env(shell: &ScopedProtocol<Shell>) {
     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<Shell>) {
-    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);
 
     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();
+    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);
 
     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();
+    let expected_fs_str = cstr16!("FS0:\\efi");
     assert_eq!(cur_fs_str, expected_fs_str);
 
     /* Test current working directory cases */
@@ -124,13 +110,13 @@ pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
     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
@@ -139,30 +125,30 @@ pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
     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)
         .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 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
         .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 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");

uefi/src/proto/shell/mod.rs

@@ -18,7 +18,7 @@ use crate::{CStr16, Char16};
 #[unsafe_protocol(ShellProtocol::GUID)]
 pub struct Shell(ShellProtocol);
 
-/// Contains environment variables
+/// Iterator over the names of environmental variables obtained from the Shell protocol.
 #[derive(Debug)]
 pub struct Vars<'a> {
     /// Char16 containing names of environment variables
@@ -29,6 +29,8 @@ pub struct Vars<'a> {
 
 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<Self::Item> {
         let cur_start = self.inner;
         let mut cur_len = 0;