Skip to content

Commit 6852357

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

File tree

2 files changed

+61
-68
lines changed

2 files changed

+61
-68
lines changed

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

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,26 @@
33
use uefi::boot::ScopedProtocol;
44
use uefi::proto::shell::Shell;
55
use uefi::{boot, cstr16};
6-
use uefi_raw::Status;
76

8-
/// Test `get_env()`, `get_envs()`, and `set_env()`
7+
/// Test `var()`, `vars()`, and `set_var()`
98
pub fn test_env(shell: &ScopedProtocol<Shell>) {
109
/* Test retrieving list of environment variable names */
11-
let mut cur_env_vec = shell.get_envs();
10+
let mut cur_env_vec = shell.vars();
1211
assert_eq!(cur_env_vec.next().unwrap(), cstr16!("path"),);
1312
// check pre-defined shell variables; see UEFI Shell spec
1413
assert_eq!(cur_env_vec.next().unwrap(), cstr16!("nonesting"),);
15-
let cur_env_vec = shell.get_envs();
14+
let cur_env_vec = shell.vars();
1615
let default_len = cur_env_vec.count();
1716

1817
/* Test setting and getting a specific environment variable */
19-
let cur_env_vec = shell.get_envs();
18+
let cur_env_vec = shell.vars();
2019
let test_var = cstr16!("test_var");
2120
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);
21+
assert!(shell.var(test_var).is_none());
22+
let status = shell.set_var(test_var, test_val, false);
23+
assert!(status.is_ok());
2524
let cur_env_str = shell
26-
.get_env(test_var)
25+
.var(test_var)
2726
.expect("Could not get environment variable");
2827
assert_eq!(cur_env_str, test_val);
2928

@@ -34,7 +33,7 @@ pub fn test_env(shell: &ScopedProtocol<Shell>) {
3433
}
3534
}
3635
assert!(!found_var);
37-
let cur_env_vec = shell.get_envs();
36+
let cur_env_vec = shell.vars();
3837
let mut found_var = false;
3938
for env_var in cur_env_vec {
4039
if env_var == test_var {
@@ -43,49 +42,49 @@ pub fn test_env(shell: &ScopedProtocol<Shell>) {
4342
}
4443
assert!(found_var);
4544

46-
let cur_env_vec = shell.get_envs();
45+
let cur_env_vec = shell.vars();
4746
assert_eq!(cur_env_vec.count(), default_len + 1);
4847

4948
/* Test deleting environment variable */
5049
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());
50+
let status = shell.set_var(test_var, test_val, false);
51+
assert!(status.is_ok());
52+
assert!(shell.var(test_var).is_none());
5453

55-
let cur_env_vec = shell.get_envs();
54+
let cur_env_vec = shell.vars();
5655
let mut found_var = false;
5756
for env_var in cur_env_vec {
5857
if env_var == test_var {
5958
found_var = true;
6059
}
6160
}
6261
assert!(!found_var);
63-
let cur_env_vec = shell.get_envs();
62+
let cur_env_vec = shell.vars();
6463
assert_eq!(cur_env_vec.count(), default_len);
6564
}
6665

67-
/// Test `get_cur_dir()` and `set_cur_dir()`
66+
/// Test `current_dir()` and `set_current_dir()`
6867
pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
6968
/* Test setting and getting current file system and current directory */
7069
let fs_var = cstr16!("fs0:");
7170
let dir_var = cstr16!("/");
72-
let status = shell.set_cur_dir(Some(fs_var), Some(dir_var));
73-
assert_eq!(status, Status::SUCCESS);
71+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
72+
assert!(status.is_ok());
7473

7574
let cur_fs_str = shell
76-
.get_cur_dir(Some(fs_var))
75+
.current_dir(Some(fs_var))
7776
.expect("Could not get the current file system mapping");
7877
let expected_fs_str = cstr16!("FS0:\\");
7978
assert_eq!(cur_fs_str, expected_fs_str);
8079

8180
// Changing current file system
8281
let fs_var = cstr16!("fs1:");
8382
let dir_var = cstr16!("/");
84-
let status = shell.set_cur_dir(Some(fs_var), Some(dir_var));
85-
assert_eq!(status, Status::SUCCESS);
83+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
84+
assert!(status.is_ok());
8685

8786
let cur_fs_str = shell
88-
.get_cur_dir(Some(fs_var))
87+
.current_dir(Some(fs_var))
8988
.expect("Could not get the current file system mapping");
9089
assert_ne!(cur_fs_str, expected_fs_str);
9190
let expected_fs_str = cstr16!("FS1:\\");
@@ -94,11 +93,11 @@ pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
9493
// Changing current file system and current directory
9594
let fs_var = cstr16!("fs0:");
9695
let dir_var = cstr16!("efi/");
97-
let status = shell.set_cur_dir(Some(fs_var), Some(dir_var));
98-
assert_eq!(status, Status::SUCCESS);
96+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
97+
assert!(status.is_ok());
9998

10099
let cur_fs_str = shell
101-
.get_cur_dir(Some(fs_var))
100+
.current_dir(Some(fs_var))
102101
.expect("Could not get the current file system mapping");
103102
assert_ne!(cur_fs_str, expected_fs_str);
104103
let expected_fs_str = cstr16!("FS0:\\efi");
@@ -108,50 +107,50 @@ pub fn test_cur_dir(shell: &ScopedProtocol<Shell>) {
108107

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

113112
// Setting the current working file system and current working directory
114113
let dir_var = cstr16!("fs0:/");
115-
let status = shell.set_cur_dir(None, Some(dir_var));
116-
assert_eq!(status, Status::SUCCESS);
114+
let status = shell.set_current_dir(None, Some(dir_var));
115+
assert!(status.is_ok());
117116
let cur_fs_str = shell
118-
.get_cur_dir(Some(fs_var))
117+
.current_dir(Some(fs_var))
119118
.expect("Could not get the current file system mapping");
120119
let expected_fs_str = cstr16!("FS0:");
121120
assert_eq!(cur_fs_str, expected_fs_str);
122121

123122
let cur_fs_str = shell
124-
.get_cur_dir(None)
123+
.current_dir(None)
125124
.expect("Could not get the current file system mapping");
126125
assert_eq!(cur_fs_str, expected_fs_str);
127126

128127
// Changing current working directory
129128
let dir_var = cstr16!("/efi");
130-
let status = shell.set_cur_dir(None, Some(dir_var));
131-
assert_eq!(status, Status::SUCCESS);
129+
let status = shell.set_current_dir(None, Some(dir_var));
130+
assert!(status.is_ok());
132131
let cur_fs_str = shell
133-
.get_cur_dir(Some(fs_var))
132+
.current_dir(Some(fs_var))
134133
.expect("Could not get the current file system mapping");
135134
let expected_fs_str = cstr16!("FS0:\\efi");
136135
assert_eq!(cur_fs_str, expected_fs_str);
137136
let cur_fs_str = shell
138-
.get_cur_dir(None)
137+
.current_dir(None)
139138
.expect("Could not get the current file system mapping");
140139
assert_eq!(cur_fs_str, expected_fs_str);
141140

142141
// Changing current directory in a non-current working file system
143142
let fs_var = cstr16!("fs0:");
144143
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);
144+
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
145+
assert!(status.is_ok());
147146
let cur_fs_str = shell
148-
.get_cur_dir(None)
147+
.current_dir(None)
149148
.expect("Could not get the current file system mapping");
150149
assert_ne!(cur_fs_str, expected_fs_str);
151150

152151
let expected_fs_str = cstr16!("FS0:\\efi\\tools");
153152
let cur_fs_str = shell
154-
.get_cur_dir(Some(fs_var))
153+
.current_dir(Some(fs_var))
155154
.expect("Could not get the current file system mapping");
156155
assert_eq!(cur_fs_str, expected_fs_str);
157156
}

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

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

87
use core::marker::PhantomData;
98
use core::ptr;
109

1110
use uefi_raw::protocol::shell::ShellProtocol;
1211

13-
use crate::{CStr16, Char16};
12+
use crate::{CStr16, Char16, Result, StatusExt};
1413

1514
/// Shell Protocol
1615
#[derive(Debug)]
@@ -32,17 +31,12 @@ impl<'a> Iterator for Vars<'a> {
3231
// We iterate a list of NUL terminated CStr16s.
3332
// The list is terminated with a double NUL.
3433
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))
34+
let s = unsafe { CStr16::from_ptr(self.inner) };
35+
if s.is_empty() {
36+
None
37+
} else {
38+
self.inner = unsafe { self.inner.add(s.num_chars() + 1) };
39+
Some(s)
4640
}
4741
}
4842
}
@@ -61,8 +55,8 @@ impl Shell {
6155
/// environment variable
6256
/// * `None` - If environment variable does not exist
6357
#[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();
58+
pub fn var(&self, name: &CStr16) -> Option<&CStr16> {
59+
let name_ptr: *const Char16 = name.as_ptr();
6660
let var_val = unsafe { (self.0.get_env)(name_ptr.cast()) };
6761
if var_val.is_null() {
6862
None
@@ -71,13 +65,9 @@ impl Shell {
7165
}
7266
}
7367

74-
/// Gets the list of environment variables
75-
///
76-
/// # Returns
77-
///
78-
/// * `Vec<env_names>` - Vector of environment variable names
68+
/// Gets an iterator over the names of all environment variables
7969
#[must_use]
80-
pub fn get_envs(&self) -> Vars {
70+
pub fn vars(&self) -> Vars<'_> {
8171
let env_ptr = unsafe { (self.0.get_env)(ptr::null()) };
8272
Vars {
8373
inner: env_ptr.cast::<Char16>(),
@@ -97,10 +87,10 @@ impl Shell {
9787
/// # Returns
9888
///
9989
/// * `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) }
90+
pub fn set_var(&self, name: &CStr16, value: &CStr16, volatile: bool) -> Result {
91+
let name_ptr: *const Char16 = name.as_ptr();
92+
let value_ptr: *const Char16 = value.as_ptr();
93+
unsafe { (self.0.set_env)(name_ptr.cast(), value_ptr.cast(), volatile) }.to_result()
10494
}
10595

10696
/// Returns the current directory on the specified device
@@ -115,8 +105,8 @@ impl Shell {
115105
/// * `Some(cwd)` - CStr16 containing the current working directory
116106
/// * `None` - Could not retrieve current directory
117107
#[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()));
108+
pub fn current_dir(&self, file_system_mapping: Option<&CStr16>) -> Option<&CStr16> {
109+
let mapping_ptr: *const Char16 = file_system_mapping.map_or(ptr::null(), CStr16::as_ptr);
120110
let cur_dir = unsafe { (self.0.get_cur_dir)(mapping_ptr.cast()) };
121111
if cur_dir.is_null() {
122112
None
@@ -140,10 +130,14 @@ impl Shell {
140130
/// # Errors
141131
///
142132
/// * `Status::EFI_NOT_FOUND` - The directory does not exist
143-
pub fn set_cur_dir(&self, file_system: Option<&CStr16>, directory: Option<&CStr16>) -> Status {
133+
pub fn set_current_dir(
134+
&self,
135+
file_system: Option<&CStr16>,
136+
directory: Option<&CStr16>,
137+
) -> Result {
144138
let fs_ptr: *const Char16 = file_system.map_or(ptr::null(), |x| (x.as_ptr()));
145139
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()) }
140+
unsafe { (self.0.set_cur_dir)(fs_ptr.cast(), dir_ptr.cast()) }.to_result()
147141
}
148142
}
149143

0 commit comments

Comments
 (0)