Skip to content

Commit 4bef474

Browse files
RenTrieuRenTrieu
committed
uefi: Revising function calls, names, and return types to better match standard convention
1 parent e0c8959 commit 4bef474

File tree

2 files changed

+34
-239
lines changed

2 files changed

+34
-239
lines changed

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

Lines changed: 23 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -3,89 +3,29 @@
33
use uefi::boot::ScopedProtocol;
44
use uefi::proto::shell::Shell;
55
use uefi::{boot, cstr16};
6-
use uefi_raw::Status;
7-
8-
/// Test `get_env()`, `get_envs()`, and `set_env()`
9-
pub fn test_env(shell: &ScopedProtocol<Shell>) {
10-
/* Test retrieving list of environment variable names */
11-
let mut cur_env_vec = shell.get_envs();
12-
assert_eq!(cur_env_vec.next().unwrap(), cstr16!("path"),);
13-
// check pre-defined shell variables; see UEFI Shell spec
14-
assert_eq!(cur_env_vec.next().unwrap(), cstr16!("nonesting"),);
15-
let cur_env_vec = shell.get_envs();
16-
let default_len = cur_env_vec.count();
17-
18-
/* Test setting and getting a specific environment variable */
19-
let cur_env_vec = shell.get_envs();
20-
let test_var = cstr16!("test_var");
21-
let test_val = cstr16!("test_val");
22-
assert!(shell.get_env(test_var).is_none());
23-
let status = shell.set_env(test_var, test_val, false);
24-
assert_eq!(status, Status::SUCCESS);
25-
let cur_env_str = shell
26-
.get_env(test_var)
27-
.expect("Could not get environment variable");
28-
assert_eq!(cur_env_str, test_val);
29-
30-
let mut found_var = false;
31-
for env_var in cur_env_vec {
32-
if env_var == test_var {
33-
found_var = true;
34-
}
35-
}
36-
assert!(!found_var);
37-
let cur_env_vec = shell.get_envs();
38-
let mut found_var = false;
39-
for env_var in cur_env_vec {
40-
if env_var == test_var {
41-
found_var = true;
42-
}
43-
}
44-
assert!(found_var);
45-
46-
let cur_env_vec = shell.get_envs();
47-
assert_eq!(cur_env_vec.count(), default_len + 1);
48-
49-
/* Test deleting environment variable */
50-
let test_val = cstr16!("");
51-
let status = shell.set_env(test_var, test_val, false);
52-
assert_eq!(status, Status::SUCCESS);
53-
assert!(shell.get_env(test_var).is_none());
54-
55-
let cur_env_vec = shell.get_envs();
56-
let mut found_var = false;
57-
for env_var in cur_env_vec {
58-
if env_var == test_var {
59-
found_var = true;
60-
}
61-
}
62-
assert!(!found_var);
63-
let cur_env_vec = shell.get_envs();
64-
assert_eq!(cur_env_vec.count(), default_len);
65-
}
666

67-
/// Test `get_cur_dir()` and `set_cur_dir()`
7+
/// Test `current_dir()` and `set_current_dir()`
688
pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
699
/* Test setting and getting current file system and current directory */
7010
let fs_var = cstr16!("fs0:");
7111
let dir_var = cstr16!("/");
72-
let status = shell.set_cur_dir(Some(fs_var), Some(dir_var));
73-
assert_eq!(status, Status::SUCCESS);
12+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
13+
assert!(status.is_ok());
7414

7515
let cur_fs_str = shell
76-
.get_cur_dir(Some(fs_var))
16+
.current_dir(Some(fs_var))
7717
.expect("Could not get the current file system mapping");
7818
let expected_fs_str = cstr16!("FS0:\\");
7919
assert_eq!(cur_fs_str, expected_fs_str);
8020

8121
// Changing current file system
8222
let fs_var = cstr16!("fs1:");
8323
let dir_var = cstr16!("/");
84-
let status = shell.set_cur_dir(Some(fs_var), Some(dir_var));
85-
assert_eq!(status, Status::SUCCESS);
24+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
25+
assert!(status.is_ok());
8626

8727
let cur_fs_str = shell
88-
.get_cur_dir(Some(fs_var))
28+
.current_dir(Some(fs_var))
8929
.expect("Could not get the current file system mapping");
9030
assert_ne!(cur_fs_str, expected_fs_str);
9131
let expected_fs_str = cstr16!("FS1:\\");
@@ -94,11 +34,11 @@ pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
9434
// Changing current file system and current directory
9535
let fs_var = cstr16!("fs0:");
9636
let dir_var = cstr16!("efi/");
97-
let status = shell.set_cur_dir(Some(fs_var), Some(dir_var));
98-
assert_eq!(status, Status::SUCCESS);
37+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
38+
assert!(status.is_ok());
9939

10040
let cur_fs_str = shell
101-
.get_cur_dir(Some(fs_var))
41+
.current_dir(Some(fs_var))
10242
.expect("Could not get the current file system mapping");
10343
assert_ne!(cur_fs_str, expected_fs_str);
10444
let expected_fs_str = cstr16!("FS0:\\efi");
@@ -108,50 +48,50 @@ pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
10848

10949
// At this point, the current working file system has not been set
11050
// So we expect a NULL output
111-
assert!(shell.get_cur_dir(None).is_none());
51+
assert!(shell.current_dir(None).is_none());
11252

11353
// Setting the current working file system and current working directory
11454
let dir_var = cstr16!("fs0:/");
115-
let status = shell.set_cur_dir(None, Some(dir_var));
116-
assert_eq!(status, Status::SUCCESS);
55+
let status = shell.set_current_dir(None, Some(dir_var));
56+
assert!(status.is_ok());
11757
let cur_fs_str = shell
118-
.get_cur_dir(Some(fs_var))
58+
.current_dir(Some(fs_var))
11959
.expect("Could not get the current file system mapping");
12060
let expected_fs_str = cstr16!("FS0:");
12161
assert_eq!(cur_fs_str, expected_fs_str);
12262

12363
let cur_fs_str = shell
124-
.get_cur_dir(None)
64+
.current_dir(None)
12565
.expect("Could not get the current file system mapping");
12666
assert_eq!(cur_fs_str, expected_fs_str);
12767

12868
// Changing current working directory
12969
let dir_var = cstr16!("/efi");
130-
let status = shell.set_cur_dir(None, Some(dir_var));
131-
assert_eq!(status, Status::SUCCESS);
70+
let status = shell.set_current_dir(None, Some(dir_var));
71+
assert!(status.is_ok());
13272
let cur_fs_str = shell
133-
.get_cur_dir(Some(fs_var))
73+
.current_dir(Some(fs_var))
13474
.expect("Could not get the current file system mapping");
13575
let expected_fs_str = cstr16!("FS0:\\efi");
13676
assert_eq!(cur_fs_str, expected_fs_str);
13777
let cur_fs_str = shell
138-
.get_cur_dir(None)
78+
.current_dir(None)
13979
.expect("Could not get the current file system mapping");
14080
assert_eq!(cur_fs_str, expected_fs_str);
14181

14282
// Changing current directory in a non-current working file system
14383
let fs_var = cstr16!("fs0:");
14484
let dir_var = cstr16!("efi/tools");
145-
let status = shell.set_cur_dir(Some(fs_var), Some(dir_var));
146-
assert_eq!(status, Status::SUCCESS);
85+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
86+
assert!(status.is_ok());
14787
let cur_fs_str = shell
148-
.get_cur_dir(None)
88+
.current_dir(None)
14989
.expect("Could not get the current file system mapping");
15090
assert_ne!(cur_fs_str, expected_fs_str);
15191

15292
let expected_fs_str = cstr16!("FS0:\\efi\\tools");
15393
let cur_fs_str = shell
154-
.get_cur_dir(Some(fs_var))
94+
.current_dir(Some(fs_var))
15595
.expect("Could not get the current file system mapping");
15696
assert_eq!(cur_fs_str, expected_fs_str);
15797
}
@@ -164,6 +104,5 @@ pub fn test() {
164104
let shell =
165105
boot::open_protocol_exclusive::<Shell>(handle).expect("Failed to open Shell protocol");
166106

167-
test_env(&shell);
168107
test_cur_dir(&shell);
169108
}

‎uefi/src/proto/shell/mod.rs‎

Lines changed: 11 additions & 155 deletions
Original file line numberDiff line numberDiff line change
@@ -3,106 +3,19 @@
33
//! EFI Shell Protocol v2.2
44
55
use uefi_macros::unsafe_protocol;
6-
use uefi_raw::Status;
76

8-
use core::marker::PhantomData;
97
use core::ptr;
108

119
use uefi_raw::protocol::shell::ShellProtocol;
1210

13-
use crate::{CStr16, Char16};
11+
use crate::{CStr16, Char16, Result, StatusExt};
1412

1513
/// Shell Protocol
1614
#[derive(Debug)]
1715
#[repr(transparent)]
1816
#[unsafe_protocol(ShellProtocol::GUID)]
1917
pub struct Shell(ShellProtocol);
20-
21-
/// Iterator over the names of environmental variables obtained from the Shell protocol.
22-
#[derive(Debug)]
23-
pub struct Vars<'a> {
24-
/// Char16 containing names of environment variables
25-
inner: *const Char16,
26-
/// Placeholder to attach a lifetime to `Vars`
27-
placeholder: PhantomData<&'a CStr16>,
28-
}
29-
30-
impl<'a> Iterator for Vars<'a> {
31-
type Item = &'a CStr16;
32-
// We iterate a list of NUL terminated CStr16s.
33-
// The list is terminated with a double NUL.
34-
fn next(&mut self) -> Option<Self::Item> {
35-
let cur_start = self.inner;
36-
let mut cur_len = 0;
37-
unsafe {
38-
if *(cur_start) == Char16::from_u16_unchecked(0) {
39-
return None;
40-
}
41-
while *(cur_start.add(cur_len)) != Char16::from_u16_unchecked(0) {
42-
cur_len += 1;
43-
}
44-
self.inner = self.inner.add(cur_len + 1);
45-
Some(CStr16::from_ptr(cur_start))
46-
}
47-
}
48-
}
49-
5018
impl Shell {
51-
/// Gets the value of the specified environment variable
52-
///
53-
/// # Arguments
54-
///
55-
/// * `name` - The environment variable name of which to retrieve the
56-
/// value.
57-
///
58-
/// # Returns
59-
///
60-
/// * `Some(<env_value>)` - &CStr16 containing the value of the
61-
/// environment variable
62-
/// * `None` - If environment variable does not exist
63-
#[must_use]
64-
pub fn get_env(&self, name: &CStr16) -> Option<&CStr16> {
65-
let name_ptr: *const Char16 = core::ptr::from_ref::<CStr16>(name).cast();
66-
let var_val = unsafe { (self.0.get_env)(name_ptr.cast()) };
67-
if var_val.is_null() {
68-
None
69-
} else {
70-
unsafe { Some(CStr16::from_ptr(var_val.cast())) }
71-
}
72-
}
73-
74-
/// Gets the list of environment variables
75-
///
76-
/// # Returns
77-
///
78-
/// * `Vec<env_names>` - Vector of environment variable names
79-
#[must_use]
80-
pub fn get_envs(&self) -> Vars {
81-
let env_ptr = unsafe { (self.0.get_env)(ptr::null()) };
82-
Vars {
83-
inner: env_ptr.cast::<Char16>(),
84-
placeholder: PhantomData,
85-
}
86-
}
87-
88-
/// Sets the environment variable
89-
///
90-
/// # Arguments
91-
///
92-
/// * `name` - The environment variable for which to set the value
93-
/// * `value` - The new value of the environment variable
94-
/// * `volatile` - Indicates whether the variable is volatile or
95-
/// not
96-
///
97-
/// # Returns
98-
///
99-
/// * `Status::SUCCESS` - The variable was successfully set
100-
pub fn set_env(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Status {
101-
let name_ptr: *const Char16 = core::ptr::from_ref::<CStr16>(name).cast();
102-
let value_ptr: *const Char16 = core::ptr::from_ref::<CStr16>(value).cast();
103-
unsafe { (self.0.set_env)(name_ptr.cast(), value_ptr.cast(), volatile) }
104-
}
105-
10619
/// Returns the current directory on the specified device
10720
///
10821
/// # Arguments
@@ -115,8 +28,8 @@ impl Shell {
11528
/// * `Some(cwd)` - CStr16 containing the current working directory
11629
/// * `None` - Could not retrieve current directory
11730
#[must_use]
118-
pub fn get_cur_dir(&self, file_system_mapping: Option<&CStr16>) -> Option<&CStr16> {
119-
let mapping_ptr: *const Char16 = file_system_mapping.map_or(ptr::null(), |x| (x.as_ptr()));
31+
pub fn current_dir(&self, file_system_mapping: Option<&CStr16>) -> Option<&CStr16> {
32+
let mapping_ptr: *const Char16 = file_system_mapping.map_or(ptr::null(), CStr16::as_ptr);
12033
let cur_dir = unsafe { (self.0.get_cur_dir)(mapping_ptr.cast()) };
12134
if cur_dir.is_null() {
12235
None
@@ -140,70 +53,13 @@ impl Shell {
14053
/// # Errors
14154
///
14255
/// * `Status::EFI_NOT_FOUND` - The directory does not exist
143-
pub fn set_cur_dir(&self, file_system: Option<&CStr16>, directory: Option<&CStr16>) -> Status {
144-
let fs_ptr: *const Char16 = file_system.map_or(ptr::null(), |x| (x.as_ptr()));
145-
let dir_ptr: *const Char16 = directory.map_or(ptr::null(), |x| (x.as_ptr()));
146-
unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) }
147-
}
148-
}
149-
150-
#[cfg(test)]
151-
mod tests {
152-
use super::*;
153-
use alloc::vec::Vec;
154-
use uefi::cstr16;
155-
156-
/// Testing Vars struct
157-
#[test]
158-
fn test_vars() {
159-
// Empty Vars
160-
let mut vars_mock = Vec::<u16>::new();
161-
vars_mock.push(0);
162-
vars_mock.push(0);
163-
let mut vars = Vars {
164-
inner: vars_mock.as_ptr().cast(),
165-
placeholder: PhantomData,
166-
};
167-
assert!(vars.next().is_none());
168-
169-
// One environment variable in Vars
170-
let mut vars_mock = Vec::<u16>::new();
171-
vars_mock.push(b'f' as u16);
172-
vars_mock.push(b'o' as u16);
173-
vars_mock.push(b'o' as u16);
174-
vars_mock.push(0);
175-
vars_mock.push(0);
176-
let vars = Vars {
177-
inner: vars_mock.as_ptr().cast(),
178-
placeholder: PhantomData,
179-
};
180-
assert_eq!(vars.collect::<Vec<_>>(), Vec::from([cstr16!("foo")]));
181-
182-
// Multiple environment variables in Vars
183-
let mut vars_mock = Vec::<u16>::new();
184-
vars_mock.push(b'f' as u16);
185-
vars_mock.push(b'o' as u16);
186-
vars_mock.push(b'o' as u16);
187-
vars_mock.push(b'1' as u16);
188-
vars_mock.push(0);
189-
vars_mock.push(b'b' as u16);
190-
vars_mock.push(b'a' as u16);
191-
vars_mock.push(b'r' as u16);
192-
vars_mock.push(0);
193-
vars_mock.push(b'b' as u16);
194-
vars_mock.push(b'a' as u16);
195-
vars_mock.push(b'z' as u16);
196-
vars_mock.push(b'2' as u16);
197-
vars_mock.push(0);
198-
vars_mock.push(0);
199-
200-
let vars = Vars {
201-
inner: vars_mock.as_ptr().cast(),
202-
placeholder: PhantomData,
203-
};
204-
assert_eq!(
205-
vars.collect::<Vec<_>>(),
206-
Vec::from([cstr16!("foo1"), cstr16!("bar"), cstr16!("baz2")])
207-
);
56+
pub fn set_current_dir(
57+
&self,
58+
file_system: Option<&CStr16>,
59+
directory: Option<&CStr16>,
60+
) -> Result {
61+
let fs_ptr: *const Char16 = file_system.map_or(ptr::null(), |x| x.as_ptr());
62+
let dir_ptr: *const Char16 = directory.map_or(ptr::null(), |x| x.as_ptr());
63+
unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) }.to_result()
20864
}
20965
}

0 commit comments

Comments
 (0)