Handle UEFI warnings more cleanly

Cargo.toml

@@ -8,6 +8,7 @@ edition = "2018"
 
 [dependencies]
 bitflags = "1"
+log = { version = "0.4", default-features = false }
 ucs2 = "0.1"
 
 [workspace]

src/error/mod.rs

@@ -1,8 +1,106 @@
 use core::result;
+use log::warn;
 
 /// Definition of UEFI's standard status codes
 mod status;
 pub use self::status::Status;
 
+/// This type is used when an UEFI operation has completed, but some non-fatal
+/// problems may have been encountered along the way
+#[must_use]
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum Completion<T> {
+    /// The operation completed without problems
+    Success(T),
+
+    /// The operation completed, but some non-fatal issues were encountered
+    Warning(T, Status),
+}
+
+impl<T> Completion<T> {
+    /// Split the completion into a (status, value) pair
+    pub fn split(self) -> (T, Status) {
+        match self {
+            Completion::Success(res) => (res, Status::SUCCESS),
+            Completion::Warning(res, stat) => (res, stat),
+        }
+    }
+
+    /// Access the inner value, logging the warning if there is any
+    pub fn value(self) -> T {
+        match self {
+            Completion::Success(res) => res,
+            Completion::Warning(res, stat) => {
+                warn!("Encountered UEFI warning: {:?}", stat);
+                res
+            }
+        }
+    }
+
+    /// Assume that no warning occured, panic if not
+    pub fn unwrap(self) -> T {
+        match self {
+            Completion::Success(res) => res,
+            Completion::Warning(_, w) => {
+                unwrap_failed("Called `Completion::unwrap()` on a `Warning` value", w)
+            }
+        }
+    }
+
+    /// Assume that no warning occured, panic with provided message if not
+    pub fn expect(self, msg: &str) -> T {
+        match self {
+            Completion::Success(res) => res,
+            Completion::Warning(_, w) => unwrap_failed(msg, w),
+        }
+    }
+
+    /// Transform the inner value without unwrapping the Completion
+    pub fn map<U>(self, f: impl Fn(T) -> U) -> Completion<U> {
+        match self {
+            Completion::Success(res) => Completion::Success(f(res)),
+            Completion::Warning(res, stat) => Completion::Warning(f(res), stat),
+        }
+    }
+
+    /// Merge this completion with a success or warning status
+    ///
+    /// Since this type only has storage for one warning, if two warnings must
+    /// be stored, one of them will be spilled into the logs.
+    ///
+    pub fn with_warning(self, extra_stat: Status) -> Self {
+        match self {
+            Completion::Success(res) => {
+                if extra_stat.is_success() {
+                    Completion::Success(res)
+                } else {
+                    Completion::Warning(res, extra_stat)
+                }
+            }
+            Completion::Warning(res, stat) => {
+                if extra_stat.is_success() {
+                    Completion::Warning(res, stat)
+                } else {
+                    warn!("Encountered UEFI warning {:?}", stat);
+                    Completion::Warning(res, extra_stat)
+                }
+            }
+        }
+    }
+}
+
+impl<T> From<T> for Completion<T> {
+    fn from(res: T) -> Self {
+        Completion::Success(res)
+    }
+}
+
+// This is a separate function to reduce the code size of the methods
+#[inline(never)]
+#[cold]
+fn unwrap_failed(msg: &str, warning: Status) -> ! {
+    panic!("{}: {:?}", msg, warning)
+}
+
 /// Return type of many UEFI functions.
-pub type Result<T> = result::Result<T, Status>;
+pub type Result<T> = result::Result<Completion<T>, Status>;

src/error/status.rs

@@ -1,4 +1,4 @@
-use super::Result;
+use super::{Completion, Result};
 use core::ops;
 use ucs2;
 
@@ -129,9 +129,10 @@ impl Status {
     where
         F: FnOnce() -> T,
     {
-        // FIXME: Is that the best way to handle warnings?
         if self.is_success() {
-            Ok(f())
+            Ok(Completion::Success(f()))
+        } else if self.is_warning() {
+            Ok(Completion::Warning(f(), self))
         } else {
             Err(self)
         }
@@ -146,7 +147,7 @@ impl Into<Result<()>> for Status {
 }
 
 impl ops::Try for Status {
-    type Ok = ();
+    type Ok = Completion<()>;
     type Error = Status;
 
     fn into_result(self) -> Result<()> {

src/lib.rs

@@ -24,8 +24,6 @@
 //! therefore all the network protocols will be unavailable.
 
 #![feature(optin_builtin_traits)]
-#![feature(min_const_fn)]
-#![feature(tool_lints)]
 #![feature(try_trait)]
 #![no_std]
 // Enable some additional warnings and lints.
@@ -37,7 +35,7 @@ mod data_types;
 pub use self::data_types::{Event, Guid, Handle};
 
 mod error;
-pub use self::error::{Result, Status};
+pub use self::error::{Completion, Result, Status};
 
 pub mod table;
 

src/proto/console/gop.rs

@@ -24,7 +24,7 @@
 //! but in practice there might be some extra padding used for efficiency.
 
 use core::{ptr, slice};
-use crate::{Result, Status};
+use crate::{Completion, Result, Status};
 
 /// Provides access to the video hardware's frame buffer.
 ///
@@ -71,7 +71,7 @@ impl GraphicsOutput {
     }
 
     /// Returns information about all available graphics modes.
-    pub fn modes<'a>(&'a self) -> impl Iterator<Item = Mode> + 'a {
+    pub fn modes<'a>(&'a self) -> impl Iterator<Item = Completion<Mode>> + 'a {
         ModeIter {
             gop: self,
             current: 0,
@@ -397,7 +397,7 @@ struct ModeIter<'a> {
 }
 
 impl<'a> Iterator for ModeIter<'a> {
-    type Item = Mode;
+    type Item = Completion<Mode>;
 
     fn next(&mut self) -> Option<Self::Item> {
         let index = self.current;

src/proto/console/pointer/mod.rs

@@ -37,9 +37,8 @@ impl Pointer {
         let mut pointer_state = unsafe { mem::uninitialized() };
 
         match (self.get_state)(self, &mut pointer_state) {
-            Status::SUCCESS => Ok(Some(pointer_state)),
-            Status::NOT_READY => Ok(None),
-            error => Err(error),
+            Status::NOT_READY => Ok(None.into()),
+            other => other.into_with(|| Some(pointer_state)),
         }
     }
 

src/proto/console/serial.rs

@@ -1,7 +1,7 @@
 //! Abstraction over byte stream devices, also known as serial I/O devices.
 
 use bitflags::bitflags;
-use crate::{Result, Status};
+use crate::{Completion, Result, Status};
 
 /// Provides access to a serial I/O device.
 ///
@@ -83,14 +83,14 @@ impl Serial {
     /// 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> {
         let mut buffer_size = data.len();
 
-        let status = (self.write)(self, &mut buffer_size, data.as_ptr());
-
-        match status {
-            Status::SUCCESS | Status::TIMEOUT => Ok(buffer_size),
-            err => Err(err),
+        match (self.write)(self, &mut buffer_size, data.as_ptr()) {
+            s @ Status::TIMEOUT => Ok(Completion::Warning(buffer_size, s)),
+            other => other.into_with(|| buffer_size),
         }
     }
 
@@ -99,14 +99,14 @@ impl Serial {
     /// 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> {
         let mut buffer_size = data.len();
 
-        let status = (self.read)(self, &mut buffer_size, data.as_mut_ptr());
-
-        match status {
-            Status::SUCCESS | Status::TIMEOUT => Ok(buffer_size),
-            err => Err(err),
+        match (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),
         }
     }
 

src/proto/console/text/input.rs

@@ -34,9 +34,8 @@ impl Input {
         let mut key = unsafe { mem::uninitialized() };
 
         match (self.read_key_stroke)(self, &mut key) {
-            Status::SUCCESS => Ok(Some(key)),
-            Status::NOT_READY => Ok(None),
-            error => Err(error),
+            Status::NOT_READY => Ok(None.into()),
+            other => other.into_with(|| Some(key)),
         }
     }
 

src/proto/console/text/output.rs

@@ -1,5 +1,5 @@
 use core::fmt;
-use crate::{Result, Status};
+use crate::{Completion, Result, Status};
 
 /// Interface for text-based output devices.
 ///
@@ -53,7 +53,7 @@ impl Output {
 
     /// Returns an iterator of all supported text modes.
     // TODO: fix the ugly lifetime parameter.
-    pub fn modes<'a>(&'a mut self) -> impl Iterator<Item = OutputMode> + 'a {
+    pub fn modes<'a>(&'a mut self) -> impl Iterator<Item = Completion<OutputMode>> + 'a {
         let max = self.data.max_mode;
         OutputModeIter {
             output: self,
@@ -74,8 +74,7 @@ impl Output {
     /// alternative to this method.
     fn query_mode(&self, index: i32) -> Result<(usize, usize)> {
         let (mut columns, mut rows) = (0, 0);
-        (self.query_mode)(self, index, &mut columns, &mut rows)?;
-        Ok((columns, rows))
+        (self.query_mode)(self, index, &mut columns, &mut rows).into_with(|| (columns, rows))
     }
 
     /// Sets a mode as current.
@@ -86,8 +85,8 @@ impl Output {
     /// Returns the the current text mode.
     pub fn current_mode(&self) -> Result<OutputMode> {
         let index = self.data.mode;
-        let dims = self.query_mode(index)?;
-        Ok(OutputMode { index, dims })
+        let dims_completion = self.query_mode(index)?;
+        Ok(dims_completion.map(|dims| OutputMode { index, dims }))
     }
 
     /// Make the cursor visible or invisible.
@@ -160,7 +159,9 @@ impl fmt::Write for Output {
             i += 1;
 
             if i == BUF_SIZE {
-                flush_buffer(&mut buf, &mut i).map_err(|_| ucs2::Error::BufferOverflow)
+                flush_buffer(&mut buf, &mut i)
+                    .map_err(|_| ucs2::Error::BufferOverflow)
+                    .map(|completion| completion.value())
             } else {
                 Ok(())
             }
@@ -178,7 +179,7 @@ impl fmt::Write for Output {
         ucs2::encode_with(s, add_ch).map_err(|_| fmt::Error)?;
 
         // Flush the remainder of the buffer
-        flush_buffer(&mut buf, &mut i)
+        flush_buffer(&mut buf, &mut i).map(|completion| completion.value())
     }
 }
 
@@ -217,15 +218,15 @@ struct OutputModeIter<'a> {
 }
 
 impl<'a> Iterator for OutputModeIter<'a> {
-    type Item = OutputMode;
+    type Item = Completion<OutputMode>;
 
     fn next(&mut self) -> Option<Self::Item> {
         let index = self.current;
         if index < self.max {
             self.current += 1;
 
-            if let Ok(dims) = self.output.query_mode(index) {
-                Some(OutputMode { index, dims })
+            if let Ok(dims_completion) = self.output.query_mode(index) {
+                Some(dims_completion.map(|dims| OutputMode { index, dims }))
             } else {
                 self.next()
             }

src/proto/media/file.rs

@@ -163,7 +163,7 @@ impl<'a> Drop for File<'a> {
     fn drop(&mut self) {
         let result: Result<()> = (self.inner.close)(self.inner).into();
         // The spec says this always succeeds.
-        result.expect("Failed to close file");
+        result.expect("Failed to close file").value();
     }
 }