workspace: streamline error types for load() functions

Author: phip1611

multiboot2-common/src/bytes_ref.rs

@@ -22,7 +22,7 @@ impl<'a, H: Header> TryFrom<&'a [u8]> for BytesRef<'a, H> {
 
     fn try_from(bytes: &'a [u8]) -> Result<Self, Self::Error> {
         if bytes.len() < mem::size_of::<H>() {
-            return Err(MemoryError::MinLengthNotSatisfied);
+            return Err(MemoryError::ShorterThanHeader);
         }
         // Doesn't work as expected: if align_of_val(&value[0]) < ALIGNMENT {
         if bytes.as_ptr().align_offset(ALIGNMENT) != 0 {
@@ -57,13 +57,13 @@ mod tests {
         let empty: &[u8] = &[];
         assert_eq!(
             BytesRef::<'_, DummyTestHeader>::try_from(empty),
-            Err(MemoryError::MinLengthNotSatisfied)
+            Err(MemoryError::ShorterThanHeader)
         );
 
         let slice = &[0_u8, 1, 2, 3, 4, 5, 6];
         assert_eq!(
             BytesRef::<'_, DummyTestHeader>::try_from(&slice[..]),
-            Err(MemoryError::MinLengthNotSatisfied)
+            Err(MemoryError::ShorterThanHeader)
         );
 
         let slice = AlignedBytes([0_u8, 1, 2, 3, 4, 5, 6, 7, 0, 0, 0]);

multiboot2-common/src/lib.rs

@@ -225,7 +225,7 @@ impl<H: Header> DynSizedStructure<H> {
         let hdr = unsafe { &*ptr };
 
         if hdr.payload_len() > bytes.len() {
-            return Err(MemoryError::InvalidHeaderSize);
+            return Err(MemoryError::InvalidReportedTotalSize);
         }
 
         // At this point we know that the memory slice fulfills the base
@@ -308,14 +308,14 @@ pub enum MemoryError {
     WrongAlignment,
     /// The memory must cover at least the length of the sized structure header
     /// type.
-    MinLengthNotSatisfied,
+    ShorterThanHeader,
     /// The buffer misses the terminating padding to the next alignment
     /// boundary. The padding is relevant to satisfy Rustc/Miri, but also the
     /// spec mandates that the padding is added.
     MissingPadding,
     /// The size-property has an illegal value that can't be fulfilled with the
     /// given bytes.
-    InvalidHeaderSize,
+    InvalidReportedTotalSize,
 }
 
 #[cfg(feature = "unstable")]

multiboot2-header/Changelog.md

@@ -13,6 +13,8 @@ place, please head to the documentation of `multiboot2-common`.
 - **Breaking** All functions that returns something useful are now `#[must_use]`
 - **Breaking** The builder type is now just called `Builder`. This needs the
   `builder` feature.
+- **Breaking:** The error type returned by `Multiboot2Header::load` has been
+  changed.
 - Updated to latest `multiboot2` dependency
 
 ## 0.4.0 (2024-05-01)

multiboot2-header/src/header.rs

@@ -4,10 +4,12 @@ use crate::{
     HeaderTagType, InformationRequestHeaderTag, ModuleAlignHeaderTag, RelocatableHeaderTag,
     TagIter,
 };
+#[cfg(feature = "unstable")]
+use core::error::Error;
 use core::fmt::{Debug, Formatter};
 use core::mem::size_of;
 use core::ptr::NonNull;
-use multiboot2_common::{DynSizedStructure, Header, Tag, ALIGNMENT};
+use multiboot2_common::{DynSizedStructure, Header, MemoryError, Tag, ALIGNMENT};
 
 /// Magic value for a [`Multiboot2Header`], as defined by the spec.
 pub const MAGIC: u32 = 0xe85250d6;
@@ -35,8 +37,8 @@ impl<'a> Multiboot2Header<'a> {
     /// This function may produce undefined behaviour, if the provided `addr` is not a valid
     /// Multiboot2 header pointer.
     pub unsafe fn load(ptr: *const Multiboot2BasicHeader) -> Result<Self, LoadError> {
-        let ptr = NonNull::new(ptr.cast_mut()).ok_or(LoadError::InvalidAddress)?;
-        let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(|_e| LoadError::TooSmall)?;
+        let ptr = NonNull::new(ptr.cast_mut()).ok_or(LoadError::Memory(MemoryError::Null))?;
+        let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(LoadError::Memory)?;
         let this = Self(inner);
 
         let header = this.0.header();
@@ -58,7 +60,7 @@ impl<'a> Multiboot2Header<'a> {
     /// If there is no header, it returns `None`.
     pub fn find_header(buffer: &[u8]) -> Result<Option<(&[u8], u32)>, LoadError> {
         if buffer.as_ptr().align_offset(ALIGNMENT) != 0 {
-            return Err(LoadError::InvalidAddress);
+            return Err(LoadError::Memory(MemoryError::WrongAlignment));
         }
 
         let mut windows = buffer[0..8192].windows(4);
@@ -70,7 +72,7 @@ impl<'a> Multiboot2Header<'a> {
                 if idx % 8 == 0 {
                     idx
                 } else {
-                    return Err(LoadError::InvalidAddress);
+                    return Err(LoadError::Memory(MemoryError::WrongAlignment));
                 }
             }
             None => return Ok(None),
@@ -87,7 +89,7 @@ impl<'a> Multiboot2Header<'a> {
         let header_length: usize = u32::from_le_bytes(
             windows
                 .next()
-                .ok_or(LoadError::TooSmall)?
+                .ok_or(LoadError::Memory(MemoryError::MissingPadding))?
                 .try_into()
                 .unwrap(), // 4 bytes are a u32
         )
@@ -221,22 +223,28 @@ impl Debug for Multiboot2Header<'_> {
     }
 }
 
-/// Errors that can occur when parsing a header from a slice.
-/// See [`Multiboot2Header::find_header`].
+/// Errors that occur when a chunk of memory can't be parsed as
+/// [`Multiboot2Header`].
 #[derive(Copy, Clone, Debug, derive_more::Display, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum LoadError {
-    /// The checksum does not match the data.
+    /// The provided checksum does not match the expected value.
     ChecksumMismatch,
-    /// The header is not properly 64-bit aligned (or a null pointer).
-    InvalidAddress,
     /// The header does not contain the correct magic number.
     MagicNotFound,
-    /// The header is truncated.
-    TooSmall,
+    /// The provided memory can't be parsed as [`Multiboot2Header`].
+    /// See [`MemoryError`].
+    Memory(MemoryError),
 }
 
 #[cfg(feature = "unstable")]
-impl core::error::Error for LoadError {}
+impl Error for LoadError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        match self {
+            Self::Memory(inner) => Some(inner),
+            _ => None,
+        }
+    }
+}
 
 /// The "basic" Multiboot2 header. This means only the properties, that are known during
 /// compile time. All other information are derived during runtime from the size property.

multiboot2/Changelog.md

@@ -18,6 +18,8 @@ place, please head to the documentation of `multiboot2-common`.
 - **Breaking:** The trait `TagTrait` was removed and was replaced by a new `Tag`
   trait coming from `multiboot2-common`. This only affects you if you provide
   custom tag types for the library.
+- **Breaking:** The error type returned by `BootInformation::load` has been
+  changed.
 
 **General Note on Safety and UB (TL;DR: Crate is Safe)**
 

multiboot2/src/boot_information.rs

@@ -8,24 +8,34 @@ use crate::{
     ElfSectionIter, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag, MemoryMapTag,
     ModuleIter, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagIter, TagType, VBEInfoTag,
 };
+#[cfg(feature = "unstable")]
+use core::error::Error;
 use core::fmt;
 use core::mem;
 use core::ptr::NonNull;
 use derive_more::Display;
 use multiboot2_common::{DynSizedStructure, Header, MaybeDynSized, MemoryError, Tag};
 
-/// Error type that describes errors while loading/parsing a multiboot2 information structure
-/// from a given address.
+/// Errors that occur when a chunk of memory can't be parsed as
+/// [`BootInformation`].
 #[derive(Display, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub enum MbiLoadError {
+pub enum LoadError {
+    /// The provided memory can't be parsed as [`BootInformation`].
     /// See [`MemoryError`].
     Memory(MemoryError),
     /// Missing mandatory end tag.
     NoEndTag,
 }
 
 #[cfg(feature = "unstable")]
-impl core::error::Error for MbiLoadError {}
+impl Error for LoadError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        match self {
+            Self::Memory(inner) => Some(inner),
+            Self::NoEndTag => None,
+        }
+    }
+}
 
 /// The basic header of a [`BootInformation`] as sized Rust type.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -91,13 +101,13 @@ impl<'a> BootInformation<'a> {
     ///   boot environments, such as UEFI.
     /// * The memory at `ptr` must not be modified after calling `load` or the
     ///   program may observe unsynchronized mutation.
-    pub unsafe fn load(ptr: *const BootInformationHeader) -> Result<Self, MbiLoadError> {
-        let ptr = NonNull::new(ptr.cast_mut()).ok_or(MbiLoadError::Memory(MemoryError::Null))?;
-        let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(MbiLoadError::Memory)?;
+    pub unsafe fn load(ptr: *const BootInformationHeader) -> Result<Self, LoadError> {
+        let ptr = NonNull::new(ptr.cast_mut()).ok_or(LoadError::Memory(MemoryError::Null))?;
+        let inner = DynSizedStructure::ref_from_ptr(ptr).map_err(LoadError::Memory)?;
 
         let this = Self(inner);
         if !this.has_valid_end_tag() {
-            return Err(MbiLoadError::NoEndTag);
+            return Err(LoadError::NoEndTag);
         }
         Ok(this)
     }

multiboot2/src/lib.rs

@@ -84,7 +84,7 @@ mod vbe_info;
 
 pub use multiboot2_common::{DynSizedStructure, MaybeDynSized, Tag};
 
-pub use boot_information::{BootInformation, BootInformationHeader, MbiLoadError};
+pub use boot_information::{BootInformation, BootInformationHeader, LoadError};
 pub use boot_loader_name::BootLoaderNameTag;
 #[cfg(feature = "builder")]
 pub use builder::Builder;
@@ -1098,7 +1098,7 @@ mod tests {
     /// This test succeeds if it compiles.
     fn mbi_load_error_implements_error() {
         fn consumer<E: core::error::Error>(_e: E) {}
-        consumer(MbiLoadError::IllegalAddress)
+        consumer(LoadError::NoEndTag)
     }
 
     /// Example for a custom tag.