multiboot2-header: streamline constructor

Author: phip1611

multiboot2-header/Changelog.md

@@ -6,6 +6,7 @@
 - **BREAKING** bumped dependency to `[email protected]`
 - **BREAKING** renamed `MULTIBOOT2_HEADER_MAGIC` to `MAGIC`
 - **BREAKING** renamed `Multiboot2HeaderBuilder` to `HeaderBuilder`
+- **BREAKING** renamed `from_addr` to `load`. The function now consumes a ptr.
 - added the optional `unstable` feature (requires nightly)
   - implement `core::error::Error` for `LoadError`
 

multiboot2-header/README.md

@@ -52,7 +52,7 @@ fn main() {
         .build();
 
     // Cast bytes in vector to Multiboot2 information structure
-    let mb2_hdr = unsafe { Multiboot2Header::from_addr(mb2_hdr_bytes.as_ptr() as usize) };
+    let mb2_hdr = unsafe { Multiboot2Header::from_addr(mb2_hdr_bytes.as_ptr().cast()) };
     println!("{:#?}", mb2_hdr);
 }
 ```

multiboot2-header/examples/minimal.rs

@@ -23,6 +23,6 @@ fn main() {
         .build();
 
     // Cast bytes in vector to Multiboot2 information structure
-    let mb2_hdr = unsafe { Multiboot2Header::from_addr(mb2_hdr_bytes.as_ptr() as usize) };
+    let mb2_hdr = unsafe { Multiboot2Header::load(mb2_hdr_bytes.as_ptr().cast()) };
     println!("{:#?}", mb2_hdr);
 }

multiboot2-header/src/builder/header.rs

@@ -281,8 +281,8 @@ mod tests {
         println!("expected_len: {} bytes", builder.expected_len());
 
         let mb2_hdr_data = builder.build();
-        let mb2_hdr = mb2_hdr_data.as_ptr() as usize;
-        let mb2_hdr = unsafe { Multiboot2Header::from_addr(mb2_hdr) }
+        let mb2_hdr = mb2_hdr_data.as_ptr().cast();
+        let mb2_hdr = unsafe { Multiboot2Header::load(mb2_hdr) }
             .expect("the generated header to be loadable");
         println!("{:#?}", mb2_hdr);
         assert_eq!(

multiboot2-header/src/header.rs

@@ -16,11 +16,10 @@ pub const MAGIC: u32 = 0xe85250d6;
 /// by all tags (see [`crate::tags::HeaderTagType`]).
 /// Use this if you get a pointer to the header and just want
 /// to parse it. If you want to construct the type by yourself,
-/// please look at [`crate::builder::HeaderBuilder`].
+/// please look at [`crate::builder::HeaderBuilder`]..
+#[derive(Debug)]
 #[repr(transparent)]
-pub struct Multiboot2Header<'a> {
-    inner: &'a Multiboot2BasicHeader,
-}
+pub struct Multiboot2Header<'a>(&'a Multiboot2BasicHeader);
 
 impl<'a> Multiboot2Header<'a> {
     /// Public constructor for this type with various validations.
@@ -35,21 +34,23 @@ impl<'a> Multiboot2Header<'a> {
     /// # Safety
     /// This function may produce undefined behaviour, if the provided `addr` is not a valid
     /// Multiboot2 header pointer.
-    // This function can be `const` on newer Rust versions.
-    #[allow(clippy::missing_const_for_fn)]
-    pub unsafe fn from_addr(addr: usize) -> Result<Self, LoadError> {
-        if addr == 0 || addr % 8 != 0 {
+    pub unsafe fn load(ptr: *const Multiboot2BasicHeader) -> Result<Self, LoadError> {
+        // null or not aligned
+        if ptr.is_null() || ptr.align_offset(8) != 0 {
             return Err(LoadError::InvalidAddress);
         }
-        let ptr = addr as *const Multiboot2BasicHeader;
+
         let reference = &*ptr;
+
         if reference.header_magic() != MAGIC {
             return Err(LoadError::MagicNotFound);
         }
+
         if !reference.verify_checksum() {
             return Err(LoadError::ChecksumMismatch);
         }
-        Ok(Self { inner: reference })
+
+        Ok(Self(reference))
     }
 
     /// Find the header in a given slice.
@@ -61,7 +62,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(4) != 0 {
-            return Err(LoadError::InvalidAddress)
+            return Err(LoadError::InvalidAddress);
         }
 
         let mut windows = buffer[0..8192].windows(4);
@@ -104,27 +105,27 @@ impl<'a> Multiboot2Header<'a> {
 
     /// Wrapper around [`Multiboot2BasicHeader::verify_checksum`].
     pub const fn verify_checksum(&self) -> bool {
-        self.inner.verify_checksum()
+        self.0.verify_checksum()
     }
     /// Wrapper around [`Multiboot2BasicHeader::header_magic`].
     pub const fn header_magic(&self) -> u32 {
-        self.inner.header_magic()
+        self.0.header_magic()
     }
     /// Wrapper around [`Multiboot2BasicHeader::arch`].
     pub const fn arch(&self) -> HeaderTagISA {
-        self.inner.arch()
+        self.0.arch()
     }
     /// Wrapper around [`Multiboot2BasicHeader::length`].
     pub const fn length(&self) -> u32 {
-        self.inner.length()
+        self.0.length()
     }
     /// Wrapper around [`Multiboot2BasicHeader::checksum`].
     pub const fn checksum(&self) -> u32 {
-        self.inner.checksum()
+        self.0.checksum()
     }
     /// Wrapper around [`Multiboot2BasicHeader::tag_iter`].
     pub fn iter(&self) -> Multiboot2HeaderTagIter {
-        self.inner.tag_iter()
+        self.0.tag_iter()
     }
     /// Wrapper around [`Multiboot2BasicHeader::calc_checksum`].
     pub const fn calc_checksum(magic: u32, arch: HeaderTagISA, length: u32) -> u32 {
@@ -192,14 +193,6 @@ impl<'a> Multiboot2Header<'a> {
     }
 }
 
-impl<'a> Debug for Multiboot2Header<'a> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
-        // For debug fmt we only output the inner field
-        let reference = unsafe { &*(self.inner as *const Multiboot2BasicHeader) };
-        Debug::fmt(reference, f)
-    }
-}
-
 /// Errors that can occur when parsing a header from a slice.
 /// See [`Multiboot2Header::find_header`].
 #[derive(Copy, Clone, Debug, derive_more::Display, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -217,9 +210,6 @@ pub enum LoadError {
 #[cfg(feature = "unstable")]
 impl core::error::Error for LoadError {}
 
-/// **Use this only if you know what you do. You probably want to use
-/// [`Multiboot2Header`] instead.**
-///
 /// 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.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -230,8 +220,8 @@ pub struct Multiboot2BasicHeader {
     arch: HeaderTagISA,
     length: u32,
     checksum: u32,
-    // additional tags..
-    // at minimum the end tag
+    // Followed by dynamic amount of dynamically sized header tags.
+    // At minimum, the end tag.
 }
 
 impl Multiboot2BasicHeader {
@@ -259,7 +249,6 @@ impl Multiboot2BasicHeader {
         (0x100000000 - magic as u64 - arch as u64 - length as u64) as u32
     }
 
-    /// Returns
     pub const fn header_magic(&self) -> u32 {
         self.header_magic
     }

multiboot2-header/src/lib.rs

@@ -25,7 +25,7 @@
 //!     .build();
 //!
 //! // Cast bytes in vector to Multiboot2 information structure
-//! let mb2_hdr = unsafe { Multiboot2Header::from_addr(mb2_hdr_bytes.as_ptr() as usize) };
+//! let mb2_hdr = unsafe { Multiboot2Header::load(mb2_hdr_bytes.as_ptr().cast()) };
 //! println!("{:#?}", mb2_hdr);
 //!
 //! ```

multiboot2/src/lib.rs

@@ -204,12 +204,8 @@ impl BootInformation<'_> {
     /// * The memory at `ptr` must not be modified after calling `load` or the
     ///   program may observe unsynchronized mutation.
     pub unsafe fn load(ptr: *const u8) -> Result<Self, MbiLoadError> {
-        if ptr.is_null() {
-            return Err(MbiLoadError::IllegalAddress);
-        }
-
-        // not aligned
-        if ptr.align_offset(8) != 0 {
+        // null or not aligned
+        if ptr.is_null() || ptr.align_offset(8) != 0 {
             return Err(MbiLoadError::IllegalAddress);
         }