Skip to content

EFI Shell Interface: CurDir Functions #1740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 97 additions & 2 deletions uefi-test-runner/src/proto/shell.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,108 @@
// SPDX-License-Identifier: MIT OR Apache-2.0

use uefi::boot;
use uefi::boot::ScopedProtocol;
use uefi::proto::shell::Shell;
use uefi::{boot, cstr16};

/// Test `current_dir()` and `set_current_dir()`
pub fn test_current_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_current_dir(Some(fs_var), Some(dir_var));
assert!(status.is_ok());

let cur_fs_str = shell
.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_current_dir(Some(fs_var), Some(dir_var));
assert!(status.is_ok());

let cur_fs_str = shell
.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:\\");
assert_eq!(cur_fs_str, expected_fs_str);

// Changing current file system and current directory
let fs_var = cstr16!("fs0:");
let dir_var = cstr16!("efi/");
let status = shell.set_current_dir(Some(fs_var), Some(dir_var));
assert!(status.is_ok());

let cur_fs_str = shell
.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");
assert_eq!(cur_fs_str, expected_fs_str);

/* Test current working directory cases */

// At this point, the current working file system has not been set
// So we expect a NULL output
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_current_dir(None, Some(dir_var));
assert!(status.is_ok());
let cur_fs_str = shell
.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
.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_current_dir(None, Some(dir_var));
assert!(status.is_ok());
let cur_fs_str = shell
.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
.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_current_dir(Some(fs_var), Some(dir_var));
assert!(status.is_ok());
let cur_fs_str = shell
.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
.current_dir(Some(fs_var))
.expect("Could not get the current file system mapping");
assert_eq!(cur_fs_str, expected_fs_str);
}

pub fn test() {
info!("Running shell protocol tests");

let handle = boot::get_handle_for_protocol::<Shell>().expect("No Shell handles");

let mut _shell =
let shell =
boot::open_protocol_exclusive::<Shell>(handle).expect("Failed to open Shell protocol");

test_current_dir(&shell);
}
60 changes: 56 additions & 4 deletions uefi/src/proto/shell/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,64 @@

//! EFI Shell Protocol v2.2

use crate::proto::unsafe_protocol;
use uefi_macros::unsafe_protocol;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not entirely consistent in this project, but I'd prefer to use crate::proto::unsafe_protocol.


pub use uefi_raw::protocol::shell::ShellProtocol;
use core::ptr;

use uefi_raw::protocol::shell::ShellProtocol;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: drop the blank lines between uses

use crate::{CStr16, Char16, Result, StatusExt};

/// Shell Protocol
#[derive(Debug)]
#[repr(transparent)]
#[unsafe_protocol(uefi_raw::protocol::shell::ShellProtocol::GUID)]
pub struct Shell(uefi_raw::protocol::shell::ShellProtocol);
#[unsafe_protocol(ShellProtocol::GUID)]
pub struct Shell(ShellProtocol);
impl Shell {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: blank line before the impl block

/// Returns the current directory on the specified device
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: period at end of sentence.

///
/// # Arguments
///
/// * `file_system_mapping` - The file system mapping for which to get
/// the current directory
///
/// # Returns
///
/// * `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> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return Result. That does mean we need to pick an error code. I think Status::NOT_FOUND should work fine.

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
} else {
unsafe { Some(CStr16::from_ptr(cur_dir.cast())) }
}
}

/// Changes the current directory on the specified device
///
/// # Arguments
///
/// * `file_system` - Pointer to the file system's mapped name.
/// * `directory` - Points to the directory on the device specified by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the Pointer/Points to. E.g. file_system - File system's mapped name.`

/// `file_system`.
///
/// # Returns
///
/// * `Status::SUCCESS` - The directory was successfully set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// * `Status::EFI_NOT_FOUND` - The directory does not exist
/// * [`Status::EFI_NOT_FOUND`] - The directory does not exist

pub fn set_current_dir(
&self,
file_system: Option<&CStr16>,
directory: Option<&CStr16>,
) -> Result {
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()) }.to_result()
}
}
Loading