Merge pull request #165 from rust-osdev/foobar

multiboot2: simplify tag_type module

multiboot2/src/end.rs

@@ -0,0 +1,39 @@
+//! Module for [`EndTag`].
+
+use crate::{Tag, TagTrait, TagType, TagTypeId};
+
+/// The end tag ends the information struct.
+#[repr(C)]
+#[derive(Debug)]
+pub struct EndTag {
+    pub typ: TagTypeId,
+    pub size: u32,
+}
+
+impl Default for EndTag {
+    fn default() -> Self {
+        Self {
+            typ: TagType::End.into(),
+            size: 8,
+        }
+    }
+}
+
+impl TagTrait for EndTag {
+    const ID: TagType = TagType::End;
+
+    fn dst_size(_base_tag: &Tag) {}
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    /// Compile time test for [`EndTag`].
+    fn test_end_tag_size() {
+        unsafe {
+            core::mem::transmute::<[u8; 8], EndTag>([0u8; 8]);
+        }
+    }
+}

multiboot2/src/lib.rs

@@ -43,19 +43,35 @@ extern crate alloc;
 #[cfg(test)]
 extern crate std;
 
-use core::fmt;
-use core::mem::size_of;
-use derive_more::Display;
-// Must be public so that custom tags can be DSTs.
+#[macro_use]
+extern crate bitflags;
+
 #[cfg(feature = "builder")]
-use crate::builder::AsBytes;
-use crate::framebuffer::UnknownFramebufferType;
+pub mod builder;
+
+mod boot_loader_name;
+mod command_line;
+mod efi;
+mod elf_sections;
+mod end;
+mod framebuffer;
+mod image_load_addr;
+mod memory_map;
+mod module;
+mod rsdp;
+mod smbios;
+mod tag;
+mod tag_trait;
+mod tag_type;
+mod vbe_info;
+
 pub use boot_loader_name::BootLoaderNameTag;
 pub use command_line::CommandLineTag;
 pub use efi::{EFIImageHandle32Tag, EFIImageHandle64Tag, EFISdt32Tag, EFISdt64Tag};
 pub use elf_sections::{
     ElfSection, ElfSectionFlags, ElfSectionIter, ElfSectionType, ElfSectionsTag,
 };
+pub use end::EndTag;
 pub use framebuffer::{FramebufferColor, FramebufferField, FramebufferTag, FramebufferType};
 pub use image_load_addr::ImageLoadPhysAddrTag;
 pub use memory_map::{
@@ -66,31 +82,22 @@ pub use module::{ModuleIter, ModuleTag};
 pub use ptr_meta::Pointee;
 pub use rsdp::{RsdpV1Tag, RsdpV2Tag};
 pub use smbios::SmbiosTag;
-use tag_type::TagIter;
-pub use tag_type::{EndTag, Tag, TagType, TagTypeId};
+pub use tag::Tag;
+pub use tag_trait::TagTrait;
+pub use tag_type::{TagType, TagTypeId};
 pub use vbe_info::{
     VBECapabilities, VBEControlInfo, VBEDirectColorAttributes, VBEField, VBEInfoTag,
     VBEMemoryModel, VBEModeAttributes, VBEModeInfo, VBEWindowAttributes,
 };
 
-#[macro_use]
-extern crate bitflags;
-
-mod boot_loader_name;
-mod command_line;
-mod efi;
-mod elf_sections;
-mod framebuffer;
-mod image_load_addr;
-mod memory_map;
-mod module;
-mod rsdp;
-mod smbios;
-mod tag_type;
-mod vbe_info;
-
+use core::fmt;
+use core::mem::size_of;
+use derive_more::Display;
+// Must be public so that custom tags can be DSTs.
 #[cfg(feature = "builder")]
-pub mod builder;
+use crate::builder::AsBytes;
+use crate::framebuffer::UnknownFramebufferType;
+use tag::TagIter;
 
 /// Magic number that a multiboot2-compliant boot loader will store in `eax` register
 /// right before handoff to the payload (the kernel). This value can be used to check,
@@ -503,61 +510,6 @@ impl fmt::Debug for BootInformation<'_> {
     }
 }
 
-/// A trait to abstract over all sized and unsized tags (DSTs). For sized tags,
-/// this trait does not much. For DSTs, a `TagTrait::dst_size` implementation
-/// must me provided, which returns the right size hint for the dynamically
-/// sized portion of the struct.
-///
-/// # Trivia
-/// This crate uses the [`Pointee`]-abstraction of the [`ptr_meta`] crate to
-/// create fat pointers for tags that are DST.
-pub trait TagTrait: Pointee {
-    /// The numeric ID of this tag.
-    const ID: TagType;
-
-    /// Returns the amount of items in the dynamically sized portion of the
-    /// DST. Note that this is not the amount of bytes. So if the dynamically
-    /// sized portion is 16 bytes in size and each element is 4 bytes big, then
-    /// this function must return 4.
-    ///
-    /// For sized tags, this just returns `()`. For DSTs, this returns an
-    /// `usize`.
-    fn dst_size(base_tag: &Tag) -> Self::Metadata;
-
-    /// Returns the tag as the common base tag structure.
-    fn as_base_tag(&self) -> &Tag {
-        let ptr = core::ptr::addr_of!(*self);
-        unsafe { &*ptr.cast::<Tag>() }
-    }
-
-    /// Returns the total size of the tag. The depends on the `size` field of
-    /// the tag.
-    fn size(&self) -> usize {
-        self.as_base_tag().size as usize
-    }
-
-    /// Returns a slice to the underlying bytes of the tag. This includes all
-    /// bytes, also for tags that are DSTs. The slice length depends on the
-    /// `size` field of the tag.
-    fn as_bytes(&self) -> &[u8] {
-        let ptr = core::ptr::addr_of!(*self);
-        unsafe { core::slice::from_raw_parts(ptr.cast(), self.size()) }
-    }
-
-    /// Creates a reference to a (dynamically sized) tag type in a safe way.
-    /// DST tags need to implement a proper [`Self::dst_size`] implementation.
-    ///
-    /// # Safety
-    /// Callers must be sure that the "size" field of the provided [`Tag`] is
-    /// sane and the underlying memory valid. The implementation of this trait
-    /// **must have** a correct [`Self::dst_size`] implementation.
-    unsafe fn from_base_tag<'a>(tag: &Tag) -> &'a Self {
-        let ptr = core::ptr::addr_of!(*tag);
-        let ptr = ptr_meta::from_raw_parts(ptr.cast(), Self::dst_size(tag));
-        &*ptr
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

multiboot2/src/tag.rs

@@ -0,0 +1,153 @@
+//! Module for the base tag definition.
+//!
+//! The relevant exports of this module is [`Tag`].
+
+use crate::{TagTrait, TagType, TagTypeId};
+use core::fmt;
+use core::fmt::{Debug, Formatter};
+use core::marker::PhantomData;
+use core::str::Utf8Error;
+
+/// Common base structure for all tags that can be passed via the Multiboot2
+/// Information Structure (MBI) to a Multiboot2 payload/program/kernel.
+///
+/// Can be transformed to any other tag (sized or unsized/DST) via
+/// [`Tag::cast_tag`].
+///
+/// Do not confuse them with the Multiboot2 header tags. They are something
+/// different.
+#[derive(Clone, Copy)]
+#[repr(C)]
+pub struct Tag {
+    pub typ: TagTypeId, // u32
+    pub size: u32,
+    // followed by additional, tag specific fields
+}
+
+impl Tag {
+    /// Returns the underlying type of the tag.
+    pub fn typ(&self) -> TagType {
+        self.typ.into()
+    }
+
+    /// Casts the base tag to the specific tag type.
+    pub fn cast_tag<'a, T: TagTrait + ?Sized + 'a>(&'a self) -> &'a T {
+        assert_eq!(self.typ, T::ID);
+        // Safety: At this point, we trust that "self.size" and the size hint
+        // for DST tags are sane.
+        unsafe { TagTrait::from_base_tag(self) }
+    }
+
+    /// Some multiboot2 tags are a DST as they end with a dynamically sized byte
+    /// slice. This function parses this slice as [`str`] so that either a valid
+    /// UTF-8 Rust string slice without a terminating null byte or an error is
+    /// returned.
+    pub fn get_dst_str_slice(bytes: &[u8]) -> Result<&str, Utf8Error> {
+        if bytes.is_empty() {
+            // Very unlikely. A sane bootloader would omit the tag in this case.
+            // But better be safe.
+            return Ok("");
+        }
+
+        // Return without a trailing null byte. By spec, the null byte should
+        // always terminate the string. However, for safety, we do make an extra
+        // check.
+        let str_slice = if bytes.ends_with(&[b'\0']) {
+            let str_len = bytes.len() - 1;
+            &bytes[0..str_len]
+        } else {
+            // Unlikely that a bootloader doesn't follow the spec and does not
+            // add a terminating null byte.
+            bytes
+        };
+        core::str::from_utf8(str_slice)
+    }
+}
+
+impl Debug for Tag {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        let tag_type = TagType::from(self.typ);
+
+        let mut debug = f.debug_struct("Tag");
+        debug.field("typ", &tag_type);
+
+        if !matches!(tag_type, TagType::Custom(_)) {
+            debug.field("typ (numeric)", &(u32::from(self.typ)));
+        }
+
+        debug.field("size", &(self.size));
+
+        debug.finish()
+    }
+}
+
+/// Iterates the MBI's tags from the first tag to the end tag.
+#[derive(Clone, Debug)]
+pub struct TagIter<'a> {
+    /// Pointer to the next tag. Updated in each iteration.
+    pub current: *const Tag,
+    /// The pointer right after the MBI. Used for additional bounds checking.
+    end_ptr_exclusive: *const u8,
+    /// Lifetime capture of the MBI's memory.
+    _mem: PhantomData<&'a ()>,
+}
+
+impl<'a> TagIter<'a> {
+    /// Creates a new iterator
+    pub fn new(mem: &'a [u8]) -> Self {
+        assert_eq!(mem.as_ptr().align_offset(8), 0);
+        TagIter {
+            current: mem.as_ptr().cast(),
+            end_ptr_exclusive: unsafe { mem.as_ptr().add(mem.len()) },
+            _mem: PhantomData,
+        }
+    }
+}
+
+impl<'a> Iterator for TagIter<'a> {
+    type Item = &'a Tag;
+
+    fn next(&mut self) -> Option<&'a Tag> {
+        // This never failed so far. But better be safe.
+        assert!(self.current.cast::<u8>() < self.end_ptr_exclusive);
+
+        let tag = unsafe { &*self.current };
+        match tag.typ() {
+            TagType::End => None, // end tag
+            _ => {
+                // We return the tag and update self.current already to the next
+                // tag.
+
+                // next pointer (rounded up to 8-byte alignment)
+                let ptr_offset = (tag.size as usize + 7) & !7;
+
+                // go to next tag
+                self.current = unsafe { self.current.cast::<u8>().add(ptr_offset).cast::<Tag>() };
+
+                Some(tag)
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_get_dst_str_slice() {
+        // unlikely case
+        assert_eq!(Ok(""), Tag::get_dst_str_slice(&[]));
+        // also unlikely case
+        assert_eq!(Ok(""), Tag::get_dst_str_slice(&[b'\0']));
+        // unlikely case: missing null byte. but the lib can cope with that
+        assert_eq!(Ok("foobar"), Tag::get_dst_str_slice("foobar".as_bytes()));
+        // test that the null bytes is not included in the string slice
+        assert_eq!(Ok("foobar"), Tag::get_dst_str_slice("foobar\0".as_bytes()));
+        // test invalid utf8
+        assert!(matches!(
+            Tag::get_dst_str_slice(&[0xff, 0xff]),
+            Err(Utf8Error { .. })
+        ));
+    }
+}

multiboot2/src/tag_trait.rs

@@ -0,0 +1,59 @@
+//! Module for [`TagTrait`].
+
+use crate::{Tag, TagType};
+use ptr_meta::Pointee;
+
+/// A trait to abstract over all sized and unsized tags (DSTs). For sized tags,
+/// this trait does not much. For DSTs, a `TagTrait::dst_size` implementation
+/// must me provided, which returns the right size hint for the dynamically
+/// sized portion of the struct.
+///
+/// # Trivia
+/// This crate uses the [`Pointee`]-abstraction of the [`ptr_meta`] crate to
+/// create fat pointers for tags that are DST.
+pub trait TagTrait: Pointee {
+    /// The numeric ID of this tag.
+    const ID: TagType;
+
+    /// Returns the amount of items in the dynamically sized portion of the
+    /// DST. Note that this is not the amount of bytes. So if the dynamically
+    /// sized portion is 16 bytes in size and each element is 4 bytes big, then
+    /// this function must return 4.
+    ///
+    /// For sized tags, this just returns `()`. For DSTs, this returns an
+    /// `usize`.
+    fn dst_size(base_tag: &Tag) -> Self::Metadata;
+
+    /// Returns the tag as the common base tag structure.
+    fn as_base_tag(&self) -> &Tag {
+        let ptr = core::ptr::addr_of!(*self);
+        unsafe { &*ptr.cast::<Tag>() }
+    }
+
+    /// Returns the total size of the tag. The depends on the `size` field of
+    /// the tag.
+    fn size(&self) -> usize {
+        self.as_base_tag().size as usize
+    }
+
+    /// Returns a slice to the underlying bytes of the tag. This includes all
+    /// bytes, also for tags that are DSTs. The slice length depends on the
+    /// `size` field of the tag.
+    fn as_bytes(&self) -> &[u8] {
+        let ptr = core::ptr::addr_of!(*self);
+        unsafe { core::slice::from_raw_parts(ptr.cast(), self.size()) }
+    }
+
+    /// Creates a reference to a (dynamically sized) tag type in a safe way.
+    /// DST tags need to implement a proper [`Self::dst_size`] implementation.
+    ///
+    /// # Safety
+    /// Callers must be sure that the "size" field of the provided [`Tag`] is
+    /// sane and the underlying memory valid. The implementation of this trait
+    /// **must have** a correct [`Self::dst_size`] implementation.
+    unsafe fn from_base_tag<'a>(tag: &Tag) -> &'a Self {
+        let ptr = core::ptr::addr_of!(*tag);
+        let ptr = ptr_meta::from_raw_parts(ptr.cast(), Self::dst_size(tag));
+        &*ptr
+    }
+}

multiboot2/src/tag_type.rs

@@ -1,20 +1,16 @@
-//! Module for the basic Multiboot2 tag and corresponding tag types.
+//! Module for tag types.
 //!
-//! The relevant exports of this module are:
-//! - [`EndTag`]
-//! - [`TagTypeId`]
-//! - [`TagType`]
-//! - [`Tag`]
-use crate::TagTrait;
+//! The relevant exports of this module are [`TagTypeId`] and [`TagType`].
+
 use core::fmt::{Debug, Formatter};
 use core::hash::Hash;
-use core::marker::PhantomData;
-use core::str::Utf8Error;
 
 /// Serialized form of [`TagType`] that matches the binary representation
 /// (`u32`). The abstraction corresponds to the `typ`/`type` field of a
 /// Multiboot2 [`Tag`]. This type can easily be created from or converted to
 /// [`TagType`].
+///
+/// [`Tag`]: crate::Tag
 #[repr(transparent)]
 #[derive(Copy, Clone, PartialOrd, PartialEq, Eq, Ord, Hash)]
 pub struct TagTypeId(u32);
@@ -290,151 +286,6 @@ 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.
-///
-/// Can be transformed to any other tag (sized or unsized/DST) via
-/// [`Tag::cast_tag`].
-///
-/// Do not confuse them with the Multiboot2 header tags. They are something
-/// different.
-#[derive(Clone, Copy)]
-#[repr(C)]
-pub struct Tag {
-    // u32
-    pub typ: TagTypeId,
-    pub size: u32,
-    // additional, tag specific fields
-}
-
-impl Tag {
-    /// Casts the base tag to the specific tag type.
-    pub fn cast_tag<'a, T: TagTrait + ?Sized + 'a>(&'a self) -> &'a T {
-        assert_eq!(self.typ, T::ID);
-        // Safety: At this point, we trust that "self.size" and the size hint
-        // for DST tags are sane.
-        unsafe { TagTrait::from_base_tag(self) }
-    }
-
-    /// Some multiboot2 tags are a DST as they end with a dynamically sized byte
-    /// slice. This function parses this slice as [`str`] so that either a valid
-    /// UTF-8 Rust string slice without a terminating null byte or an error is
-    /// returned.
-    pub fn get_dst_str_slice(bytes: &[u8]) -> Result<&str, Utf8Error> {
-        if bytes.is_empty() {
-            // Very unlikely. A sane bootloader would omit the tag in this case.
-            // But better be safe.
-            return Ok("");
-        }
-
-        // Return without a trailing null byte. By spec, the null byte should
-        // always terminate the string. However, for safety, we do make an extra
-        // check.
-        let str_slice = if bytes.ends_with(&[b'\0']) {
-            let str_len = bytes.len() - 1;
-            &bytes[0..str_len]
-        } else {
-            // Unlikely that a bootloader doesn't follow the spec and does not
-            // add a terminating null byte.
-            bytes
-        };
-        core::str::from_utf8(str_slice)
-    }
-}
-
-impl Debug for Tag {
-    fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
-        let tag_type = TagType::from(self.typ);
-
-        let mut debug = f.debug_struct("Tag");
-        debug.field("typ", &tag_type);
-
-        if !matches!(tag_type, TagType::Custom(_)) {
-            debug.field("typ (numeric)", &(u32::from(self.typ)));
-        }
-
-        debug.field("size", &(self.size));
-
-        debug.finish()
-    }
-}
-
-/// The end tag ends the information struct.
-#[repr(C)]
-#[derive(Debug)]
-pub struct EndTag {
-    pub typ: TagTypeId,
-    pub size: u32,
-}
-
-impl Default for EndTag {
-    fn default() -> Self {
-        Self {
-            typ: TagType::End.into(),
-            size: 8,
-        }
-    }
-}
-
-impl TagTrait for EndTag {
-    const ID: TagType = TagType::End;
-
-    fn dst_size(_base_tag: &Tag) {}
-}
-
-/// Iterates the MBI's tags from the first tag to the end tag.
-#[derive(Clone, Debug)]
-pub struct TagIter<'a> {
-    /// Pointer to the next tag. Updated in each iteration.
-    pub current: *const Tag,
-    /// The pointer right after the MBI. Used for additional bounds checking.
-    end_ptr_exclusive: *const u8,
-    /// Lifetime capture of the MBI's memory.
-    _mem: PhantomData<&'a ()>,
-}
-
-impl<'a> TagIter<'a> {
-    /// Creates a new iterator
-    pub fn new(mem: &'a [u8]) -> Self {
-        assert_eq!(mem.as_ptr().align_offset(8), 0);
-        TagIter {
-            current: mem.as_ptr().cast(),
-            end_ptr_exclusive: unsafe { mem.as_ptr().add(mem.len()) },
-            _mem: PhantomData,
-        }
-    }
-}
-
-impl<'a> Iterator for TagIter<'a> {
-    type Item = &'a Tag;
-
-    fn next(&mut self) -> Option<&'a Tag> {
-        // This never failed so far. But better be safe.
-        assert!(self.current.cast::<u8>() < self.end_ptr_exclusive);
-
-        let tag = unsafe { &*self.current };
-        match tag {
-            &Tag {
-                // END-Tag
-                typ: TagTypeId(0),
-                size: 8,
-            } => None, // end tag
-            tag => {
-                // We return the tag and update self.current already to the next
-                // tag.
-
-                // next pointer (rounded up to 8-byte alignment)
-                let ptr_offset = (tag.size as usize + 7) & !7;
-
-                // go to next tag
-                self.current = unsafe { self.current.cast::<u8>().add(ptr_offset).cast::<Tag>() };
-
-                Some(tag)
-            }
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -509,29 +360,4 @@ mod tests {
             assert_eq!(tag_type_from_id, tag_type_from_u16)
         }
     }
-
-    #[test]
-    fn test_get_dst_str_slice() {
-        // unlikely case
-        assert_eq!(Ok(""), Tag::get_dst_str_slice(&[]));
-        // also unlikely case
-        assert_eq!(Ok(""), Tag::get_dst_str_slice(&[b'\0']));
-        // unlikely case: missing null byte. but the lib can cope with that
-        assert_eq!(Ok("foobar"), Tag::get_dst_str_slice("foobar".as_bytes()));
-        // test that the null bytes is not included in the string slice
-        assert_eq!(Ok("foobar"), Tag::get_dst_str_slice("foobar\0".as_bytes()));
-        // test invalid utf8
-        assert!(matches!(
-            Tag::get_dst_str_slice(&[0xff, 0xff]),
-            Err(Utf8Error { .. })
-        ));
-    }
-
-    #[test]
-    /// Compile time test for [`EndTag`].
-    fn test_end_tag_size() {
-        unsafe {
-            core::mem::transmute::<[u8; 8], EndTag>([0u8; 8]);
-        }
-    }
 }