Additional error data in Result + some API cleanup and clarification (#78)

* Rename the module holding the Result more logically

* Rewrite Completion as a struct

* Add support for error payloads

* Clarify result module documentation

* Allow error data to depend on the error status

* Clean up usage of Result

- Replace Result<()> with Result
- Use new facility for cleanly reporting error data
- Fix a few incorrect APIs (bad lifetime, error/warning confusion)
- Drop some usage of core::result::Result, qualify the rest better

* Fix lifetime names

* Add warning support to test_string

* Apply rustfmt

* Plural is probably best after all

* Move background color sanity check to an assertion

Author: HadrienG2

src/data_types/strs.rs

@@ -24,7 +24,7 @@ pub struct CStr8([Char8]);
 
 impl CStr8 {
     /// Wraps a raw UEFI string with a safe C string wrapper
-    pub unsafe fn from_ptr<'a>(ptr: *const Char8) -> &'a Self {
+    pub unsafe fn from_ptr<'ptr>(ptr: *const Char8) -> &'ptr Self {
         let mut len = 0;
         while *ptr.add(len) != NUL_8 {
             len += 1
@@ -77,7 +77,7 @@ pub struct CStr16([Char16]);
 
 impl CStr16 {
     /// Wraps a raw UEFI string with a safe C string wrapper
-    pub unsafe fn from_ptr<'a>(ptr: *const Char16) -> &'a Self {
+    pub unsafe fn from_ptr<'ptr>(ptr: *const Char16) -> &'ptr Self {
         let mut len = 0;
         while *ptr.add(len) != NUL_16 {
             len += 1

src/error/mod.rs

@@ -36,8 +36,8 @@ pub mod data_types;
 pub(crate) use self::data_types::{unsafe_guid, Identify};
 pub use self::data_types::{CStr16, CStr8, Char16, Char8, Event, Guid, Handle};
 
-mod error;
-pub use self::error::{Completion, Result, ResultExt, Status};
+mod result;
+pub use self::result::{Completion, Result, ResultExt, ResultExt2, Status};
 
 pub mod table;
 

src/lib.rs

@@ -2,7 +2,7 @@
 //!
 //! This includes the system table types, `Status` codes, etc.
 
-pub use crate::{ResultExt, Status};
+pub use crate::{ResultExt, ResultExt2, Status};
 
 // Import the basic table types.
 pub use crate::table::boot::BootServices;

src/prelude.rs

@@ -68,7 +68,7 @@ impl<'boot> GraphicsOutput<'boot> {
         let mut info_sz = 0;
         let mut info = ptr::null();
 
-        (self.query_mode)(self, index, &mut info_sz, &mut info).into_with(|| {
+        (self.query_mode)(self, index, &mut info_sz, &mut info).into_with_val(|| {
             let info = unsafe { *info };
             Mode {
                 index,
@@ -79,7 +79,7 @@ impl<'boot> GraphicsOutput<'boot> {
     }
 
     /// Returns information about all available graphics modes.
-    pub fn modes<'a>(&'a self) -> impl Iterator<Item = Completion<Mode>> + 'a {
+    pub fn modes<'gop>(&'gop self) -> impl Iterator<Item = Completion<Mode>> + 'gop {
         ModeIter {
             gop: self,
             current: 0,
@@ -91,14 +91,14 @@ impl<'boot> GraphicsOutput<'boot> {
     /// of the output display to black.
     ///
     /// This function will invalidate the current framebuffer.
-    pub fn set_mode(&mut self, mode: &Mode) -> Result<()> {
+    pub fn set_mode(&mut self, mode: &Mode) -> Result {
         (self.set_mode)(self, mode.index).into()
     }
 
     /// Performs a blt (block transfer) operation on the frame buffer.
     ///
     /// Every operation requires different parameters.
-    pub fn blt(&mut self, op: BltOp) -> Result<()> {
+    pub fn blt(&mut self, op: BltOp) -> Result {
         // Demultiplex the operation type.
         unsafe {
             match op {
@@ -286,13 +286,13 @@ impl<'boot> GraphicsOutput<'boot> {
 }
 
 #[repr(C)]
-struct ModeData<'a> {
+struct ModeData<'info> {
     // Number of modes which the GOP supports.
     max_mode: u32,
     // Current mode.
     mode: u32,
     // Information about the current mode.
-    info: &'a ModeInfo,
+    info: &'info ModeInfo,
     // Size of the above structure.
     info_sz: usize,
     // Physical address of the frame buffer.
@@ -401,13 +401,13 @@ impl ModeInfo {
 }
 
 /// Iterator for graphics modes.
-struct ModeIter<'a> {
-    gop: &'a GraphicsOutput<'a>,
+struct ModeIter<'gop> {
+    gop: &'gop GraphicsOutput<'gop>,
     current: u32,
     max: u32,
 }
 
-impl<'a> Iterator for ModeIter<'a> {
+impl<'gop> Iterator for ModeIter<'gop> {
     type Item = Completion<Mode>;
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -479,7 +479,7 @@ pub enum BltRegion {
 
 /// Blit operation to perform.
 #[derive(Debug)]
-pub enum BltOp<'a> {
+pub enum BltOp<'buf> {
     /// Fills a rectangle of video display with a pixel color.
     VideoFill {
         /// The color to fill with.
@@ -492,7 +492,7 @@ pub enum BltOp<'a> {
     /// Reads data from the video display to the buffer.
     VideoToBltBuffer {
         /// Buffer into which to copy data.
-        buffer: &'a mut [BltPixel],
+        buffer: &'buf mut [BltPixel],
         /// Coordinates of the source rectangle, in the frame buffer.
         src: (usize, usize),
         /// Location of the destination rectangle in the user-provided buffer
@@ -504,7 +504,7 @@ pub enum BltOp<'a> {
     /// Delta must be the stride (count of bytes in a row) of the buffer.
     BufferToVideo {
         /// Buffer from which to copy data.
-        buffer: &'a [BltPixel],
+        buffer: &'buf [BltPixel],
         /// Location of the source rectangle in the user-provided buffer.
         src: BltRegion,
         /// Coordinates of the destination rectangle, in the frame buffer.
@@ -525,13 +525,13 @@ pub enum BltOp<'a> {
 }
 
 /// Direct access to a memory-mapped frame buffer
-pub struct FrameBuffer<'a> {
+pub struct FrameBuffer<'gop> {
     base: *mut u8,
     size: usize,
-    _lifetime: PhantomData<&'a mut u8>,
+    _lifetime: PhantomData<&'gop mut u8>,
 }
 
-impl<'a> FrameBuffer<'a> {
+impl<'gop> FrameBuffer<'gop> {
     /// Access the raw framebuffer pointer
     ///
     /// To use this pointer safely and correctly, you must...

src/proto/console/gop.rs

@@ -24,7 +24,7 @@ impl<'boot> Pointer<'boot> {
     /// # Errors
     ///
     /// - `DeviceError` if the device is malfunctioning and cannot be reset.
-    pub fn reset(&mut self, extended_verification: bool) -> Result<()> {
+    pub fn reset(&mut self, extended_verification: bool) -> Result {
         (self.reset)(self, extended_verification).into()
     }
 
@@ -41,7 +41,7 @@ impl<'boot> Pointer<'boot> {
 
         match (self.get_state)(self, &mut pointer_state) {
             Status::NOT_READY => Ok(None.into()),
-            other => other.into_with(|| Some(pointer_state)),
+            other => other.into_with_val(|| Some(pointer_state)),
         }
     }
 

src/proto/console/pointer/mod.rs

@@ -1,7 +1,7 @@
 //! Abstraction over byte stream devices, also known as serial I/O devices.
 
 use crate::proto::Protocol;
-use crate::{unsafe_guid, Completion, Result, Status};
+use crate::{unsafe_guid, Result, Status};
 use bitflags::bitflags;
 
 /// Provides access to a serial I/O device.
@@ -37,7 +37,7 @@ pub struct Serial<'boot> {
 
 impl<'boot> Serial<'boot> {
     /// Reset the device.
-    pub fn reset(&mut self) -> Result<()> {
+    pub fn reset(&mut self) -> Result {
         (self.reset)(self).into()
     }
 
@@ -59,7 +59,7 @@ impl<'boot> Serial<'boot> {
     /// - if either `baud_rate` or `receive_fifo_depth` is less than
     ///   the device's minimum, an error will be returned;
     ///   this value will be rounded down to the nearest value supported by the device;
-    pub fn set_attributes(&mut self, mode: &IoMode) -> Result<()> {
+    pub fn set_attributes(&mut self, mode: &IoMode) -> Result {
         (self.set_attributes)(
             self,
             mode.baud_rate,
@@ -75,47 +75,41 @@ impl<'boot> Serial<'boot> {
     /// Retrieve the device's current control bits.
     pub fn get_control_bits(&self) -> Result<ControlBits> {
         let mut bits = ControlBits::empty();
-        (self.get_control_bits)(self, &mut bits).into_with(|| bits)
+        (self.get_control_bits)(self, &mut bits).into_with_val(|| bits)
     }
 
     /// Sets the device's new control bits.
     ///
     /// Not all bits can be modified with this function. A mask of the allowed
     /// bits is stored in the [`ControlBits::SETTABLE`] constant.
-    pub fn set_control_bits(&mut self, bits: ControlBits) -> Result<()> {
+    pub fn set_control_bits(&mut self, bits: ControlBits) -> Result {
         (self.set_control_bits)(self, bits).into()
     }
 
     /// Reads data from this device.
     ///
-    /// Returns the number of bytes actually read from the device.
-    /// In the case of a timeout or buffer overrun, this number will be smaller
-    /// than the buffer's size.
-    ///
-    /// Unlike UEFI, we handle timeouts as warnings, not errors
-    pub fn read(&mut self, data: &mut [u8]) -> Result<usize> {
+    /// This operation will block until the buffer has been filled with data or
+    /// an error occurs. In the latter case, the error will indicate how many
+    /// bytes were actually read from the device.
+    pub fn read(&mut self, data: &mut [u8]) -> Result<(), usize> {
         let mut buffer_size = data.len();
-
-        match unsafe { (self.read)(self, &mut buffer_size, data.as_mut_ptr()) } {
-            s @ Status::TIMEOUT => Ok(Completion::Warning(buffer_size, s)),
-            other => other.into_with(|| buffer_size),
-        }
+        unsafe { (self.read)(self, &mut buffer_size, data.as_mut_ptr()) }.into_with(
+            || debug_assert_eq!(buffer_size, data.len()),
+            |_| buffer_size,
+        )
     }
 
     /// Writes data to this device.
     ///
-    /// Returns the number of bytes actually written to the device.
-    /// In the case of a timeout, this number will be smaller than
-    /// the buffer's size.
-    ///
-    /// Unlike UEFI, we handle timeouts as warnings, not errors
-    pub fn write(&mut self, data: &[u8]) -> Result<usize> {
+    /// This operation will block until the data has been fully written or an
+    /// error occurs. In the latter case, the error will indicate how many bytes
+    /// were actually written to the device.
+    pub fn write(&mut self, data: &[u8]) -> Result<(), usize> {
         let mut buffer_size = data.len();
-
-        match unsafe { (self.write)(self, &mut buffer_size, data.as_ptr()) } {
-            s @ Status::TIMEOUT => Ok(Completion::Warning(buffer_size, s)),
-            other => other.into_with(|| buffer_size),
-        }
+        unsafe { (self.write)(self, &mut buffer_size, data.as_ptr()) }.into_with(
+            || debug_assert_eq!(buffer_size, data.len()),
+            |_| buffer_size,
+        )
     }
 }
 

src/proto/console/serial.rs

@@ -21,7 +21,7 @@ impl Input {
     /// # Errors
     ///
     /// - `DeviceError` if the device is malfunctioning and cannot be reset.
-    pub fn reset(&mut self, extended_verification: bool) -> Result<()> {
+    pub fn reset(&mut self, extended_verification: bool) -> Result {
         (self.reset)(self, extended_verification).into()
     }
 
@@ -38,7 +38,7 @@ impl Input {
 
         match (self.read_key_stroke)(self, &mut key) {
             Status::NOT_READY => Ok(None.into()),
-            other => other.into_with(|| Some(key.into())),
+            other => other.into_with_val(|| Some(key.into())),
         }
     }