From d5124c768a62da4da7bc78a5dc3a804d2e323220 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Mon, 21 Nov 2022 21:33:08 +0100 Subject: [PATCH 1/2] typos --- uefi/src/data_types/strs.rs | 2 +- uefi/src/proto/console/text/output.rs | 2 +- uefi/src/proto/media/file/mod.rs | 2 +- xtask/src/device_path/node.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/uefi/src/data_types/strs.rs b/uefi/src/data_types/strs.rs index 437f6d9a9..fdd2cdd3c 100644 --- a/uefi/src/data_types/strs.rs +++ b/uefi/src/data_types/strs.rs @@ -337,7 +337,7 @@ impl CStr16 { } /// Writes each [`Char16`] as a [`char`] (4 bytes long in Rust language) into the buffer. - /// It is up the the implementer of [`core::fmt::Write`] to convert the char to a string + /// It is up to the implementer of [`core::fmt::Write`] to convert the char to a string /// with proper encoding/charset. For example, in the case of [`alloc::string::String`] /// all Rust chars (UTF-32) get converted to UTF-8. /// diff --git a/uefi/src/proto/console/text/output.rs b/uefi/src/proto/console/text/output.rs index beb69d752..366241ecc 100644 --- a/uefi/src/proto/console/text/output.rs +++ b/uefi/src/proto/console/text/output.rs @@ -110,7 +110,7 @@ impl<'boot> Output<'boot> { (self.query_mode)(self, index, &mut columns, &mut rows).into_with_val(|| (columns, rows)) } - /// Returns the the current text mode. + /// Returns the current text mode. pub fn current_mode(&self) -> Result> { match self.data.mode { -1 => Ok(None), diff --git a/uefi/src/proto/media/file/mod.rs b/uefi/src/proto/media/file/mod.rs index 88d504ae9..c512f78c6 100644 --- a/uefi/src/proto/media/file/mod.rs +++ b/uefi/src/proto/media/file/mod.rs @@ -182,7 +182,7 @@ pub trait File: Sized { fn is_directory(&self) -> Result; } -// Internal File helper methods to access the funciton pointer table. +// Internal File helper methods to access the function pointer table. trait FileInternal: File { fn imp(&mut self) -> &mut FileImpl { unsafe { &mut *self.handle().0 } diff --git a/xtask/src/device_path/node.rs b/xtask/src/device_path/node.rs index 560975707..cc9ec9d86 100644 --- a/xtask/src/device_path/node.rs +++ b/xtask/src/device_path/node.rs @@ -165,7 +165,7 @@ impl Node { } // It's not trivial to nicely format the DST data since - // the the slice might be unaligned. Treat it as a byte + // the slice might be unaligned. Treat it as a byte // slice instead. quote!({ let ptr = addr_of!(#field_val); From d295e8de6552fe2fa06ae31ccc74419f5250120b Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Thu, 24 Nov 2022 09:21:52 +0100 Subject: [PATCH 2/2] documentation and code improvements for Status, Error, and read() --- uefi/src/proto/media/file/dir.rs | 10 ++++++---- uefi/src/proto/media/file/mod.rs | 13 +++++++++++++ uefi/src/proto/media/file/regular.rs | 9 +++++++-- uefi/src/result/mod.rs | 4 ++-- uefi/src/result/status.rs | 21 ++++++++++++++++++--- 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/uefi/src/proto/media/file/dir.rs b/uefi/src/proto/media/file/dir.rs index 98b38344a..ae323bb28 100644 --- a/uefi/src/proto/media/file/dir.rs +++ b/uefi/src/proto/media/file/dir.rs @@ -50,11 +50,13 @@ impl Directory { FileInfo::assert_aligned(buffer); // Read the directory entry into the aligned storage - self.0.read(buffer).map(|size| { - if size != 0 { - unsafe { Some(FileInfo::from_uefi(buffer.as_mut_ptr().cast::())) } - } else { + self.0.read(buffer).map(|read_bytes| { + // 0 read bytes signals that the last directory entry was read + let last_directory_entry_read = read_bytes == 0; + if last_directory_entry_read { None + } else { + unsafe { Some(FileInfo::from_uefi(buffer.as_mut_ptr().cast::())) } } }) } diff --git a/uefi/src/proto/media/file/mod.rs b/uefi/src/proto/media/file/mod.rs index c512f78c6..86bb780d2 100644 --- a/uefi/src/proto/media/file/mod.rs +++ b/uefi/src/proto/media/file/mod.rs @@ -286,6 +286,19 @@ pub(super) struct FileImpl { ) -> Status, close: extern "efiapi" fn(this: &mut FileImpl) -> Status, delete: extern "efiapi" fn(this: &mut FileImpl) -> Status, + /// # Read from Regular Files + /// If `self` is not a directory, the function reads the requested number of bytes from the file + /// at the file’s current position and returns them in `buffer`. If the read goes beyond the end + /// of the file, the read length is truncated to the end of the file. The file’s current + /// position is increased by the number of bytes returned. + /// + /// # Read from Directory + /// If `self` is a directory, the function reads the directory entry at the file’s current + /// position and returns the entry in `buffer`. If the `buffer` is not large enough to hold the + /// current directory entry, then `EFI_BUFFER_TOO_SMALL` is returned and the current file + /// position is not updated. `buffer_size` is set to be the size of the buffer needed to read + /// the entry. On success, the current position is updated to the next directory entry. If there + /// are no more directory entries, the read returns a zero-length buffer. read: unsafe extern "efiapi" fn( this: &mut FileImpl, buffer_size: &mut usize, diff --git a/uefi/src/proto/media/file/regular.rs b/uefi/src/proto/media/file/regular.rs index 819612ebf..6c09c8398 100644 --- a/uefi/src/proto/media/file/regular.rs +++ b/uefi/src/proto/media/file/regular.rs @@ -21,7 +21,7 @@ impl RegularFile { Self(handle) } - /// Read data from file + /// Read data from file. /// /// Try to read as much as possible into `buffer`. Returns the number of bytes that were /// actually read. @@ -38,10 +38,15 @@ impl RegularFile { /// and the required buffer size is provided as output. pub fn read(&mut self, buffer: &mut [u8]) -> Result> { let mut buffer_size = buffer.len(); - unsafe { (self.imp().read)(self.imp(), &mut buffer_size, buffer.as_mut_ptr()) }.into_with( + let status = + unsafe { (self.imp().read)(self.imp(), &mut buffer_size, buffer.as_mut_ptr()) }; + + status.into_with( || buffer_size, |s| { if s == Status::BUFFER_TOO_SMALL { + // `buffer_size` was updated to the required buffer size by the underlying read + // function. Some(buffer_size) } else { None diff --git a/uefi/src/result/mod.rs b/uefi/src/result/mod.rs index 32b29d285..907767e18 100644 --- a/uefi/src/result/mod.rs +++ b/uefi/src/result/mod.rs @@ -14,8 +14,8 @@ pub use self::status::Status; /// Almost all UEFI operations provide a status code as an output which /// indicates either success, a warning, or an error. This type alias maps /// [`Status::SUCCESS`] to the `Ok` variant (with optional `Output` data), and -/// maps both warning and error statuses to the `Err` variant (with optional -/// `ErrData`). +/// maps both warning and error statuses to the `Err` variant of type [`Error`], +/// which may carry optional inner `ErrData`. /// /// Warnings are treated as errors by default because they generally indicate /// an abnormal situation. diff --git a/uefi/src/result/status.rs b/uefi/src/result/status.rs index d4e1edeea..c3b4a73eb 100644 --- a/uefi/src/result/status.rs +++ b/uefi/src/result/status.rs @@ -12,6 +12,12 @@ newtype_enum! { /// enum, as injecting an unknown value in a Rust enum is undefined behaviour. /// /// For lack of a better option, we therefore model them as a newtype of usize. +/// +/// For a convenient integration into the Rust ecosystem, there are multiple +/// factory methods to convert a Status into a [`uefi::Result`]: +/// - [`Status::into_with`] +/// - [`Status::into_with_val`] +/// - [`Status::into_with_err`] #[must_use] pub enum Status: usize => { /// The operation completed successfully. @@ -122,7 +128,10 @@ impl Status { self.0 & ERROR_BIT != 0 } - /// Converts this status code into a result with a given value. + /// Converts this status code into a [`uefi::Result`] with a given `Ok` value. + /// + /// If the status does not indicate success, the status representing the specific error + /// code is embedded into the `Err` variant of type [`uefi::Error`]. #[inline] pub fn into_with_val(self, val: impl FnOnce() -> T) -> Result { if self.is_success() { @@ -132,7 +141,10 @@ impl Status { } } - /// Converts this status code into a result with a given error payload + /// Converts this status code into a [`uefi::Result`] with a given `Err` payload. + /// + /// If the status does not indicate success, the status representing the specific error + /// code is embedded into the `Err` variant of type [`uefi::Error`]. #[inline] pub fn into_with_err( self, @@ -145,7 +157,10 @@ impl Status { } } - /// Convert this status code into a result with a given value and error payload + /// Convert this status code into a result with a given `Ok` value and `Err` payload. + /// + /// If the status does not indicate success, the status representing the specific error + /// code is embedded into the `Err` variant of type [`uefi::Error`]. #[inline] pub fn into_with( self,