multiboot2: code cleanup

Author: phip1611

.github/workflows/qa.yml

@@ -9,4 +9,4 @@ jobs:
     steps:
       - uses: actions/checkout@v3
       # Executes "typos ."
-      - uses: crate-ci/typos@v1.13.20
+      - uses: crate-ci/typos@v1.15.2

multiboot2-header/Changelog.md

@@ -1,12 +1,13 @@
 # CHANGELOG for crate `multiboot2-header`
 
 ## 0.3.0 (xxxx-xx-xx)
-- MSRV is 1.68.0
-- renamed the `std` feature to `alloc`
+- **BREAKING** MSRV is 1.68.0
+- **BREAKING** renamed the `std` feature to `alloc`
+- **BREAKING** bumped dependency to `[email protected]`
+- **BREAKING** renamed `MULTIBOOT2_HEADER_MAGIC` to `MAGIC`
+- **BREAKING** renamed `Multiboot2HeaderBuilder` to `HeaderBuilder`
 - added the optional `unstable` feature (requires nightly)
   - implement `core::error::Error` for `LoadError`
-- depends on `[email protected]`
-- **BREAKING** renamed `Multiboot2HeaderBuilder` to `HeaderBuilder`
 
 ## 0.2.0 (2022-05-03)
 - **BREAKING** renamed `EntryHeaderTag` to `EntryAddressHeaderTag`

multiboot2-header/src/builder/information_request.rs

@@ -22,7 +22,7 @@ pub struct InformationRequestHeaderTagBuilder {
 #[cfg(feature = "builder")]
 impl InformationRequestHeaderTagBuilder {
     /// New builder.
-    pub fn new(flag: HeaderTagFlag) -> Self {
+    pub const fn new(flag: HeaderTagFlag) -> Self {
         Self {
             irs: BTreeSet::new(),
             flag,

multiboot2-header/src/builder/traits.rs

@@ -58,7 +58,7 @@ mod tests {
             c: u128,
         }
         impl StructAsBytes for Foobar {}
-        #[allow(clippy::blacklisted_name)]
+        #[allow(clippy::disallowed_names)]
         let foo = Foobar {
             a: 11,
             b: 22,

multiboot2-header/src/header.rs

@@ -8,8 +8,8 @@ use core::convert::TryInto;
 use core::fmt::{Debug, Formatter};
 use core::mem::size_of;
 
-/// Magic value for a [`Multiboot2Header`], as defined in spec.
-pub const MULTIBOOT2_HEADER_MAGIC: u32 = 0xe85250d6;
+/// Magic value for a [`Multiboot2Header`], as defined by the spec.
+pub const MAGIC: u32 = 0xe85250d6;
 
 /// Wrapper type around a pointer to the Multiboot2 header.
 /// The Multiboot2 header is the [`Multiboot2BasicHeader`] followed
@@ -43,7 +43,7 @@ impl<'a> Multiboot2Header<'a> {
         }
         let ptr = addr as *const Multiboot2BasicHeader;
         let reference = &*ptr;
-        if reference.header_magic() != MULTIBOOT2_HEADER_MAGIC {
+        if reference.header_magic() != MAGIC {
             return Err(LoadError::MagicNotFound);
         }
         if !reference.verify_checksum() {
@@ -56,16 +56,18 @@ impl<'a> Multiboot2Header<'a> {
     ///
     /// If it succeeds, it returns a tuple consisting of the subslice containing
     /// just the header and the index of the header in the given slice.
-    /// If it fails (either because the header is not properply 64-bit aligned
+    /// If it fails (either because the header is not properly 64-bit aligned
     /// or because it is truncated), it returns a [`LoadError`].
     /// If there is no header, it returns `None`.
     pub fn find_header(buffer: &[u8]) -> Result<Option<(&[u8], u32)>, LoadError> {
-        // the magic is 32 bit aligned and inside the first 8192 bytes
-        assert!(buffer.len() >= 8192);
+        if buffer.as_ptr().align_offset(4) != 0 {
+            return Err(LoadError::InvalidAddress)
+        }
+
         let mut windows = buffer[0..8192].windows(4);
         let magic_index = match windows.position(|vals| {
             u32::from_le_bytes(vals.try_into().unwrap()) // yes, there's 4 bytes here
-            == MULTIBOOT2_HEADER_MAGIC
+            == MAGIC
         }) {
             Some(idx) => {
                 if idx % 8 == 0 {
@@ -223,7 +225,7 @@ impl core::error::Error for LoadError {}
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 pub struct Multiboot2BasicHeader {
-    /// Must be the value of [`MULTIBOOT2_HEADER_MAGIC`].
+    /// Must be the value of [`MAGIC`].
     header_magic: u32,
     arch: HeaderTagISA,
     length: u32,
@@ -236,7 +238,7 @@ impl Multiboot2BasicHeader {
     #[cfg(feature = "builder")]
     /// Constructor for the basic header.
     pub(crate) const fn new(arch: HeaderTagISA, length: u32) -> Self {
-        let magic = MULTIBOOT2_HEADER_MAGIC;
+        let magic = MAGIC;
         let checksum = Self::calc_checksum(magic, arch, length);
         Multiboot2BasicHeader {
             header_magic: magic,

multiboot2-header/src/information_request.rs

@@ -65,7 +65,7 @@ impl<const N: usize> InformationRequestHeaderTag<N> {
     }
 
     /// Returns an [`InformationRequestHeaderTagIter`].
-    pub fn req_iter(&self) -> InformationRequestHeaderTagIter {
+    pub const fn req_iter(&self) -> InformationRequestHeaderTagIter {
         let base_struct_size = size_of::<InformationRequestHeaderTag<0>>();
         let count = self.dynamic_requests_size();
         let base_ptr = self as *const InformationRequestHeaderTag<N>;

multiboot2/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "multiboot2"
 description = """
-Library that helps you to parse the multiboot information structure (mbi) from
+Library that assists parsing the Multiboot2 Information Structure (MBI) from
 Multiboot2-compliant bootloaders, like GRUB. It supports all tags from the specification
 including full support for the sections of ELF-64. This library is `no_std` and can be
 used in a Multiboot2-kernel.

multiboot2/README.md

@@ -3,7 +3,7 @@
 [![crates.io](https://img.shields.io/crates/v/multiboot2.svg)](https://crates.io/crates/multiboot2)
 [![docs](https://docs.rs/multiboot2/badge.svg)](https://docs.rs/multiboot2/)
 
-Rust library that helps you to parse the multiboot information structure (mbi) from
+Rust library that assists parsing the Multiboot2 Information Structure (MBI) from
 Multiboot2-compliant bootloaders, like GRUB. It supports all tags from the specification
 including full support for the sections of ELF-64 files. This library is `no_std` and can be
 used in a Multiboot2-kernel.

multiboot2/src/lib.rs

@@ -9,7 +9,7 @@
 #![allow(rustdoc::private_doc_tests)]
 // --- END STYLE CHECKS ---
 
-//! Library that helps you to parse the multiboot information structure (mbi) from
+//! Library that assists parsing the Multiboot2 Information Structure (MBI) from
 //! Multiboot2-compliant bootloaders, like GRUB. It supports all tags from the specification
 //! including full support for the sections of ELF-64. This library is `no_std` and can be
 //! used in a Multiboot2-kernel.
@@ -139,6 +139,42 @@ pub enum MbiLoadError {
 #[cfg(feature = "unstable")]
 impl core::error::Error for MbiLoadError {}
 
+#[repr(C, align(8))]
+struct BootInformationInner {
+    total_size: u32,
+    _reserved: u32,
+    // followed by various, dynamically sized multiboot2 tags
+    tags: [Tag; 0],
+}
+
+impl BootInformationInner {
+    #[cfg(feature = "builder")]
+    fn new(total_size: u32) -> Self {
+        Self {
+            total_size,
+            _reserved: 0,
+            tags: [],
+        }
+    }
+
+    fn has_valid_end_tag(&self) -> bool {
+        let end_tag_prototype = EndTag::default();
+
+        let self_ptr = self as *const _;
+        let end_tag_addr = self_ptr as usize + (self.total_size - end_tag_prototype.size) as usize;
+        let end_tag = unsafe { &*(end_tag_addr as *const Tag) };
+
+        end_tag.typ == end_tag_prototype.typ && end_tag.size == end_tag_prototype.size
+    }
+}
+
+#[cfg(feature = "builder")]
+impl StructAsBytes for BootInformationInner {
+    fn byte_size(&self) -> usize {
+        core::mem::size_of::<Self>()
+    }
+}
+
 /// A Multiboot 2 Boot Information (MBI) accessor.
 #[repr(transparent)]
 pub struct BootInformation<'a>(&'a BootInformationInner);
@@ -191,35 +227,7 @@ impl BootInformation<'_> {
 
         Ok(Self(mbi))
     }
-}
-
-#[repr(C, align(8))]
-struct BootInformationInner {
-    total_size: u32,
-    _reserved: u32,
-    // followed by various, dynamically sized multiboot2 tags
-    tags: [Tag; 0],
-}
-
-impl BootInformationInner {
-    #[cfg(feature = "builder")]
-    fn new(total_size: u32) -> Self {
-        Self {
-            total_size,
-            _reserved: 0,
-            tags: [],
-        }
-    }
-}
-
-#[cfg(feature = "builder")]
-impl StructAsBytes for BootInformationInner {
-    fn byte_size(&self) -> usize {
-        core::mem::size_of::<Self>()
-    }
-}
 
-impl BootInformation<'_> {
     /// Get the start address of the boot info.
     pub fn start_address(&self) -> usize {
         core::ptr::addr_of!(*self.0) as usize
@@ -438,18 +446,6 @@ impl BootInformation<'_> {
     }
 }
 
-impl BootInformationInner {
-    fn has_valid_end_tag(&self) -> bool {
-        let end_tag_prototype = EndTag::default();
-
-        let self_ptr = self as *const _;
-        let end_tag_addr = self_ptr as usize + (self.total_size - end_tag_prototype.size) as usize;
-        let end_tag = unsafe { &*(end_tag_addr as *const Tag) };
-
-        end_tag.typ == end_tag_prototype.typ && end_tag.size == end_tag_prototype.size
-    }
-}
-
 // SAFETY: BootInformation contains a const ptr to memory that is never mutated.
 // Sending this pointer to other threads is sound.
 unsafe impl Send for BootInformation<'_> {}

multiboot2/src/tag_type.rs

@@ -287,7 +287,7 @@ mod partial_eq_impls {
 }
 
 /// Common base structure for all tags that can be passed via the Multiboot2
-/// information structure (MBI) to a Multiboot2 payload/program/kernel.
+/// Information Structure (MBI) to a Multiboot2 payload/program/kernel.
 ///
 /// Can be transformed to any other tag (sized or unsized/DST) via
 /// [`Tag::cast_tag`].