documentation and code improvements for Status, Error, and read()

Author: phip1611

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::<c_void>())) }
-            } 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::<c_void>())) }
             }
         })
     }

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,

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<usize, Option<usize>> {
         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

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.

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<T>(self, val: impl FnOnce() -> T) -> Result<T, ()> {
         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<ErrData: Debug>(
         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<T, ErrData: Debug>(
         self,