EFI Shell Interface: CurDir Functions #1740
Conversation
uefi/src/proto/shell/mod.rs
Outdated
| @@ -2,12 +2,141 @@ | |||
|
|
|||
| //! EFI Shell Protocol v2.2 | |||
|
|
|||
| use crate::proto::unsafe_protocol; | |||
| #![cfg(feature = "alloc")] | |||
There was a problem hiding this comment.
For module-wide restrictions, we prefer:
#[cfg(feature = "alloc")]
mod shell;
on the upper level instead.
There was a problem hiding this comment.
also it doesn't look to me that set_dir and get_dir need alloc at all
uefi/src/proto/shell/mod.rs
Outdated
| /// * `Some(Vec<env_names>)` - Vector of environment variable names | ||
| /// * `None` - Environment variable doesn't exist | ||
| #[must_use] | ||
| pub fn get_env<'a>(&'a self, name: Option<&CStr16>) -> Option<Vec<&'a CStr16>> { |
There was a problem hiding this comment.
i think this accidentally hit the PR.? should be part of #1679 ?
This commit implements wrappers for the following EFI Shell Protocol functions: set_cur_dir() and get_cur_dir().
This commit includes tests for the following EFI Shell Protocol functions: get_cur_dir() and set_cur_dir().
…h standard convention
RenTrieu
force-pushed
the
enhancement/efi_shell_interface_cur_dir
branch
from
4bef474 to
873e382
Compare
|
Hi Philipp. Thanks for catching these! It looks like when rebasing the commits, although I removed references to the other PR from the commit messages I neglected to do so from the code in all but the most recent commit. I've addressed it now and will also do so shortly for #1679. |
| pub struct Shell(uefi_raw::protocol::shell::ShellProtocol); | ||
| #[unsafe_protocol(ShellProtocol::GUID)] | ||
| pub struct Shell(ShellProtocol); | ||
| impl Shell { |
There was a problem hiding this comment.
nit: blank line before the impl block
| use core::ptr; | ||
|
|
||
| use uefi_raw::protocol::shell::ShellProtocol; | ||
|
|
There was a problem hiding this comment.
nit: drop the blank lines between uses
| @@ -2,12 +2,64 @@ | |||
|
|
|||
| //! EFI Shell Protocol v2.2 | |||
|
|
|||
| use crate::proto::unsafe_protocol; | |||
| use uefi_macros::unsafe_protocol; | |||
There was a problem hiding this comment.
We're not entirely consistent in this project, but I'd prefer to use crate::proto::unsafe_protocol.
| /// * `Some(cwd)` - CStr16 containing the current working directory | ||
| /// * `None` - Could not retrieve current directory | ||
| #[must_use] | ||
| pub fn current_dir(&self, file_system_mapping: Option<&CStr16>) -> Option<&CStr16> { |
There was a problem hiding this comment.
This should return Result. That does mean we need to pick an error code. I think Status::NOT_FOUND should work fine.
| #[unsafe_protocol(ShellProtocol::GUID)] | ||
| pub struct Shell(ShellProtocol); | ||
| impl Shell { | ||
| /// Returns the current directory on the specified device |
There was a problem hiding this comment.
nit: period at end of sentence.
| /// # Arguments | ||
| /// | ||
| /// * `file_system` - Pointer to the file system's mapped name. | ||
| /// * `directory` - Points to the directory on the device specified by |
There was a problem hiding this comment.
Drop the Pointer/Points to. E.g. file_system - File system's mapped name.`
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `Status::SUCCESS` - The directory was successfully set |
There was a problem hiding this comment.
Drop the Returns section. It's understood that Ok is returned in the successful case.
| /// | ||
| /// # Errors | ||
| /// | ||
| /// * `Status::EFI_NOT_FOUND` - The directory does not exist |
There was a problem hiding this comment.
| /// * `Status::EFI_NOT_FOUND` - The directory does not exist | |
| /// * [`Status::EFI_NOT_FOUND`] - The directory does not exist |
This PR implements the wrappers for the
GetCurDirandSetCurDirEFI Shell Protocol functions as part of the effort to address #448 .Checklist