Merge pull request #112 from rust-osdev/multiboot2-header-packed-n-fix

multiboot2-header: remove `packed(8)`

Author: phip1611

multiboot2-header/Changelog.md

@@ -1,5 +1,8 @@
 # CHANGELOG for crate `multiboot2-header`
 
+## v0.2.0 (2022-05-03)
+- **breaking** renamed `EntryHeaderTag` to `EntryAddressHeaderTag`
+
 ## v0.1.1 (2022-05-02)
 - fixed a bug that prevented the usage of the crate in `no_std` environments
 - added a new default `builder`-feature to Cargo which requires the `alloc`-crate

multiboot2-header/src/address.rs

@@ -6,7 +6,7 @@ use core::mem::size_of;
 /// other format. Required for legacy boot (BIOS).
 /// Determines load addresses.
 #[derive(Copy, Clone, Debug)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct AddressHeaderTag {
     typ: HeaderTagType,
     flags: HeaderTagFlag,
@@ -64,3 +64,16 @@ impl AddressHeaderTag {
         self.bss_end_addr
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::AddressHeaderTag;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(
+            core::mem::size_of::<AddressHeaderTag>(),
+            2 + 2 + 4 + 4 + 4 + 4 + 4
+        );
+    }
+}

multiboot2-header/src/builder/header.rs

@@ -4,9 +4,9 @@ use crate::builder::information_request::InformationRequestHeaderTagBuilder;
 use crate::builder::traits::StructAsBytes;
 use crate::HeaderTagISA;
 use crate::{
-    AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EndHeaderTag, EntryEfi32HeaderTag,
-    EntryEfi64HeaderTag, EntryHeaderTag, FramebufferHeaderTag, ModuleAlignHeaderTag,
-    Multiboot2BasicHeader, RelocatableHeaderTag,
+    AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EndHeaderTag,
+    EntryAddressHeaderTag, EntryEfi32HeaderTag, EntryEfi64HeaderTag, FramebufferHeaderTag,
+    ModuleAlignHeaderTag, Multiboot2BasicHeader, RelocatableHeaderTag,
 };
 use alloc::vec::Vec;
 use core::mem::size_of;
@@ -22,7 +22,7 @@ pub struct Multiboot2HeaderBuilder {
     // second
     address_tag: Option<AddressHeaderTag>,
     // third
-    entry_tag: Option<EntryHeaderTag>,
+    entry_tag: Option<EntryAddressHeaderTag>,
     // fourth
     console_tag: Option<ConsoleHeaderTag>,
     // fifth
@@ -86,7 +86,7 @@ impl Multiboot2HeaderBuilder {
             len += Self::size_or_up_aligned(size_of::<AddressHeaderTag>())
         }
         if self.entry_tag.is_some() {
-            len += Self::size_or_up_aligned(size_of::<EntryHeaderTag>())
+            len += Self::size_or_up_aligned(size_of::<EntryAddressHeaderTag>())
         }
         if self.console_tag.is_some() {
             len += Self::size_or_up_aligned(size_of::<ConsoleHeaderTag>())
@@ -192,7 +192,7 @@ impl Multiboot2HeaderBuilder {
         self.address_tag = Some(address_tag);
         self
     }
-    pub const fn entry_tag(mut self, entry_tag: EntryHeaderTag) -> Self {
+    pub const fn entry_tag(mut self, entry_tag: EntryAddressHeaderTag) -> Self {
         self.entry_tag = Some(entry_tag);
         self
     }

multiboot2-header/src/builder/traits.rs

@@ -1,9 +1,9 @@
 //! Module for the helper trait [`StructAsBytes`].
 
 use crate::{
-    AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EndHeaderTag, EntryEfi32HeaderTag,
-    EntryEfi64HeaderTag, EntryHeaderTag, FramebufferHeaderTag, InformationRequestHeaderTag,
-    ModuleAlignHeaderTag, Multiboot2BasicHeader, RelocatableHeaderTag,
+    AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EndHeaderTag,
+    EntryAddressHeaderTag, EntryEfi32HeaderTag, EntryEfi64HeaderTag, FramebufferHeaderTag,
+    InformationRequestHeaderTag, ModuleAlignHeaderTag, Multiboot2BasicHeader, RelocatableHeaderTag,
 };
 use core::mem::size_of;
 
@@ -39,7 +39,7 @@ impl StructAsBytes for ConsoleHeaderTag {}
 impl StructAsBytes for EndHeaderTag {}
 impl StructAsBytes for EntryEfi32HeaderTag {}
 impl StructAsBytes for EntryEfi64HeaderTag {}
-impl StructAsBytes for EntryHeaderTag {}
+impl StructAsBytes for EntryAddressHeaderTag {}
 impl StructAsBytes for FramebufferHeaderTag {}
 impl StructAsBytes for InformationRequestHeaderTag<0> {}
 impl StructAsBytes for ModuleAlignHeaderTag {}

multiboot2-header/src/console.rs

@@ -14,7 +14,7 @@ pub enum ConsoleHeaderTagFlags {
 /// Tells that a console must be available in MBI.
 /// Only relevant for legacy BIOS.
 #[derive(Copy, Clone, Debug)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct ConsoleHeaderTag {
     typ: HeaderTagType,
     flags: HeaderTagFlag,
@@ -48,32 +48,10 @@ impl ConsoleHeaderTag {
 
 #[cfg(test)]
 mod tests {
-    use crate::{ConsoleHeaderTag, ConsoleHeaderTagFlags, HeaderTagFlag, HeaderTagType};
-    use std::mem::size_of_val;
+    use crate::ConsoleHeaderTag;
 
-    /// Checks if rust aligns the type correctly and still "pack" all properties.
-    /// This test is necessary, because Rust doesn't support "packed" together with "align()" yet.
-    /// It seems like "packed(N)" does the right thing tho.
-    ///
-    /// This test is representative for all header tags, because all use the "packed(8)" attribute.
     #[test]
-    fn test_alignment_and_size() {
-        let tag = ConsoleHeaderTag::new(
-            HeaderTagFlag::Required,
-            ConsoleHeaderTagFlags::ConsoleRequired,
-        );
-        let ptr = get_ptr!(tag, ConsoleHeaderTag);
-        let is_aligned = ptr % 8 == 0;
-        assert!(is_aligned);
-        // 2x u16, 2x u32
-        assert_eq!(2 + 2 + 4 + 4, size_of_val(&tag));
-
-        assert_eq!(ptr + 0, get_field_ptr!(tag, typ, HeaderTagType));
-        assert_eq!(ptr + 2, get_field_ptr!(tag, flags, HeaderTagFlag));
-        assert_eq!(ptr + 4, get_field_ptr!(tag, size, u32));
-        assert_eq!(
-            ptr + 8,
-            get_field_ptr!(tag, console_flags, ConsoleHeaderTagFlags)
-        );
+    fn test_assert_size() {
+        assert_eq!(core::mem::size_of::<ConsoleHeaderTag>(), 2 + 2 + 4 + 4);
     }
 }

multiboot2-header/src/end.rs

@@ -1,10 +1,9 @@
 use crate::{HeaderTagFlag, HeaderTagType};
 use core::mem::size_of;
 
-/// Terminates a list of optional tags
-/// in a Multiboot2 header.
+/// Terminates a list of optional tags in a Multiboot2 header.
 #[derive(Copy, Clone, Debug)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct EndHeaderTag {
     // u16 value
     typ: HeaderTagType,
@@ -32,3 +31,13 @@ impl EndHeaderTag {
         self.size
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::EndHeaderTag;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(core::mem::size_of::<EndHeaderTag>(), 2 + 2 + 4);
+    }
+}

multiboot2-header/src/entry_header.rs renamed to multiboot2-header/src/entry_address.rs

@@ -4,20 +4,19 @@ use core::fmt::{Debug, Formatter};
 use core::mem::size_of;
 
 /// Specifies the physical address to which the boot loader should jump in
-/// order to start running the operating system.
-/// Not needed for ELF files.
+/// order to start running the operating system. Not needed for ELF files.
 #[derive(Copy, Clone)]
-#[repr(C, packed(8))]
-pub struct EntryHeaderTag {
+#[repr(C)]
+pub struct EntryAddressHeaderTag {
     typ: HeaderTagType,
     flags: HeaderTagFlag,
     size: u32,
     entry_addr: u32,
 }
 
-impl EntryHeaderTag {
+impl EntryAddressHeaderTag {
     pub const fn new(flags: HeaderTagFlag, entry_addr: u32) -> Self {
-        EntryHeaderTag {
+        EntryAddressHeaderTag {
             typ: HeaderTagType::EntryAddress,
             flags,
             size: size_of::<Self>() as u32,
@@ -39,13 +38,23 @@ impl EntryHeaderTag {
     }
 }
 
-impl Debug for EntryHeaderTag {
+impl Debug for EntryAddressHeaderTag {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        f.debug_struct("EntryHeaderTag")
+        f.debug_struct("EntryAddressHeaderTag")
             .field("type", &{ self.typ })
             .field("flags", &{ self.flags })
             .field("size", &{ self.size })
             .field("entry_addr", &(self.entry_addr as *const u32))
             .finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::EntryAddressHeaderTag;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(core::mem::size_of::<EntryAddressHeaderTag>(), 2 + 2 + 4 + 4);
+    }
+}

multiboot2-header/src/entry_efi_32.rs

@@ -6,8 +6,11 @@ use core::mem::size_of;
 /// This tag is taken into account only on EFI i386 platforms when Multiboot2 image header
 /// contains EFI boot services tag. Then entry point specified in ELF header and the entry address
 /// tag of Multiboot2 header are ignored.
+///
+/// Technically, this is equivalent to the [`crate::EntryAddressHeaderTag`] but with a different
+/// [`crate::HeaderTagType`].
 #[derive(Copy, Clone)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct EntryEfi32HeaderTag {
     typ: HeaderTagType,
     flags: HeaderTagFlag,
@@ -49,3 +52,13 @@ impl Debug for EntryEfi32HeaderTag {
             .finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::EntryEfi32HeaderTag;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(core::mem::size_of::<EntryEfi32HeaderTag>(), 2 + 2 + 4 + 4);
+    }
+}

multiboot2-header/src/entry_efi_64.rs

@@ -6,8 +6,11 @@ use core::mem::size_of;
 /// This tag is taken into account only on EFI amd64 platforms when Multiboot2 image header
 /// contains EFI boot services tag. Then entry point specified in ELF header and the entry address
 /// tag of Multiboot2 header are ignored.
+///
+/// Technically, this is equivalent to the [`crate::EntryAddressHeaderTag`] but with a different
+/// [`crate::HeaderTagType`].
 #[derive(Copy, Clone)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct EntryEfi64HeaderTag {
     typ: HeaderTagType,
     flags: HeaderTagFlag,
@@ -49,3 +52,13 @@ impl Debug for EntryEfi64HeaderTag {
             .finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::EntryEfi64HeaderTag;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(core::mem::size_of::<EntryEfi64HeaderTag>(), 2 + 2 + 4 + 4);
+    }
+}

multiboot2-header/src/framebuffer.rs

@@ -6,7 +6,7 @@ use core::mem::size_of;
 /// has framebuffer support. Note: This is only a
 /// recommended mode. Only relevant on legacy BIOS.
 #[derive(Copy, Clone, Debug)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct FramebufferHeaderTag {
     typ: HeaderTagType,
     flags: HeaderTagFlag,
@@ -47,3 +47,16 @@ impl FramebufferHeaderTag {
         self.depth
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::FramebufferHeaderTag;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(
+            core::mem::size_of::<FramebufferHeaderTag>(),
+            2 + 2 + 4 + 4 + 4 + 4
+        );
+    }
+}

multiboot2-header/src/header.rs

@@ -1,7 +1,7 @@
 use crate::{
-    AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EndHeaderTag, EntryEfi32HeaderTag,
-    EntryEfi64HeaderTag, EntryHeaderTag, FramebufferHeaderTag, HeaderTag, HeaderTagISA,
-    HeaderTagType, InformationRequestHeaderTag, RelocatableHeaderTag,
+    AddressHeaderTag, ConsoleHeaderTag, EfiBootServiceHeaderTag, EndHeaderTag,
+    EntryAddressHeaderTag, EntryEfi32HeaderTag, EntryEfi64HeaderTag, FramebufferHeaderTag,
+    HeaderTag, HeaderTagISA, HeaderTagType, InformationRequestHeaderTag, RelocatableHeaderTag,
 };
 use core::fmt::{Debug, Formatter};
 use core::mem::size_of;
@@ -103,7 +103,7 @@ impl<'a> Debug for Multiboot2Header<'a> {
 /// 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)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct Multiboot2BasicHeader {
     /// Must be the value of [`MULTIBOOT2_HEADER_MAGIC`].
     header_magic: u32,
@@ -290,7 +290,7 @@ impl Debug for Multiboot2HeaderTagIter {
                 let entry = &*(entry);
                 debug.entry(entry);
             } else if typ == HeaderTagType::EntryAddress {
-                let entry = t as *const EntryHeaderTag;
+                let entry = t as *const EntryAddressHeaderTag;
                 let entry = &*(entry);
                 debug.entry(entry);
             } else if typ == HeaderTagType::ConsoleFlags {
@@ -324,3 +324,13 @@ impl Debug for Multiboot2HeaderTagIter {
         debug.finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::Multiboot2BasicHeader;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(core::mem::size_of::<Multiboot2BasicHeader>(), 4 + 4 + 4 + 4);
+    }
+}

multiboot2-header/src/information_request.rs

@@ -8,7 +8,7 @@ use core::mem::size_of;
 /// Specifies what specific tag types the bootloader should provide
 /// inside the mbi.
 #[derive(Copy, Clone)]
-#[repr(C, packed(8))]
+#[repr(C)]
 pub struct InformationRequestHeaderTag<const N: usize> {
     typ: HeaderTagType,
     flags: HeaderTagFlag,
@@ -130,3 +130,24 @@ impl<'a> Debug for InformationRequestHeaderTagIter<'a> {
         debug.finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::InformationRequestHeaderTag;
+
+    #[test]
+    fn test_assert_size() {
+        assert_eq!(
+            core::mem::size_of::<InformationRequestHeaderTag<0>>(),
+            2 + 2 + 4 + 0 * 4
+        );
+        assert_eq!(
+            core::mem::size_of::<InformationRequestHeaderTag<1>>(),
+            2 + 2 + 4 + 1 * 4
+        );
+        assert_eq!(
+            core::mem::size_of::<InformationRequestHeaderTag<2>>(),
+            2 + 2 + 4 + 2 * 4
+        );
+    }
+}