Quellcode durchsuchen

Merge pull request #166 from rust-osdev/foobar

multiboot2: cleanup builder module
Philipp Schuster vor 1 Jahr
Ursprung
Commit
5de87ae144
3 geänderte Dateien mit 153 neuen und 152 gelöschten Zeilen
  1. 142 0
      multiboot2/src/builder/boxed_dst.rs
  2. 1 12
      multiboot2/src/builder/information.rs
  3. 10 140
      multiboot2/src/builder/mod.rs

+ 142 - 0
multiboot2/src/builder/boxed_dst.rs

@@ -0,0 +1,142 @@
+//! Module for [`BoxedDst`].
+
+use crate::{Tag, TagTrait, TagTypeId};
+use alloc::alloc::alloc;
+use core::alloc::Layout;
+use core::marker::PhantomData;
+use core::mem::size_of;
+use core::ops::Deref;
+use core::ptr::NonNull;
+
+/// A helper type to create boxed DST, i.e., tags with a dynamic size for the
+/// builder. This is tricky in Rust. This type behaves similar to the regular
+/// `Box` type except that it ensure the same layout is used for the (explicit)
+/// allocation and the (implicit) deallocation of memory. Otherwise, I didn't
+/// found any way to figure out the right layout for a DST. Miri always reported
+/// issues that the deallocation used a wrong layout.
+///
+/// Technically, I'm certain this code is memory safe. But with this type, I
+/// also can convince miri that it is.
+#[derive(Debug, Eq)]
+pub struct BoxedDst<T: ?Sized> {
+    ptr: core::ptr::NonNull<T>,
+    layout: Layout,
+    // marker: I used this only as the regular Box impl also does it.
+    _marker: PhantomData<T>,
+}
+
+impl<T: TagTrait<Metadata = usize> + ?Sized> BoxedDst<T> {
+    /// Create a boxed tag with the given content.
+    ///
+    /// # Parameters
+    /// - `content` - All payload bytes of the DST tag without the tag type or
+    ///               the size. The memory is only read and can be discarded
+    ///               afterwards.
+    pub(crate) fn new(content: &[u8]) -> Self {
+        // Currently, I do not find a nice way of making this dynamic so that
+        // also miri is guaranteed to be happy. But it seems that 4 is fine
+        // here. I do have control over allocation and deallocation.
+        const ALIGN: usize = 4;
+
+        let tag_size = size_of::<TagTypeId>() + size_of::<u32>() + content.len();
+
+        // By using miri, I could figure out that there often are problems where
+        // miri thinks an allocation is smaller then necessary. Most probably
+        // due to not packed structs. Using packed structs however
+        // (especially with DSTs), is a crazy ass pain and unusable :/ Therefore,
+        // the best solution I can think of is to allocate a few byte more than
+        // necessary. I think that during runtime, everything works fine and
+        // that no memory issues are present.
+        let alloc_size = (tag_size + 7) & !7; // align to next 8 byte boundary
+        let layout = Layout::from_size_align(alloc_size, ALIGN).unwrap();
+        let ptr = unsafe { alloc(layout) };
+        assert!(!ptr.is_null());
+
+        // write tag content to memory
+        unsafe {
+            // write tag type
+            let ptrx = ptr.cast::<TagTypeId>();
+            ptrx.write(T::ID.into());
+
+            // write tag size
+            let ptrx = ptrx.add(1).cast::<u32>();
+            ptrx.write(tag_size as u32);
+
+            // write rest of content
+            let ptrx = ptrx.add(1).cast::<u8>();
+            let tag_content_slice = core::slice::from_raw_parts_mut(ptrx, content.len());
+            for (i, &byte) in content.iter().enumerate() {
+                tag_content_slice[i] = byte;
+            }
+        }
+
+        let base_tag = unsafe { &*ptr.cast::<Tag>() };
+        let raw: *mut T = ptr_meta::from_raw_parts_mut(ptr.cast(), T::dst_size(base_tag));
+
+        Self {
+            ptr: NonNull::new(raw).unwrap(),
+            layout,
+            _marker: PhantomData,
+        }
+    }
+}
+
+impl<T: ?Sized> Drop for BoxedDst<T> {
+    fn drop(&mut self) {
+        unsafe { alloc::alloc::dealloc(self.ptr.as_ptr().cast(), self.layout) }
+    }
+}
+
+impl<T: ?Sized> Deref for BoxedDst<T> {
+    type Target = T;
+    fn deref(&self) -> &Self::Target {
+        unsafe { self.ptr.as_ref() }
+    }
+}
+
+impl<T: ?Sized + PartialEq> PartialEq for BoxedDst<T> {
+    fn eq(&self, other: &Self) -> bool {
+        self.deref().eq(other.deref())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::TagType;
+
+    const METADATA_SIZE: usize = 8;
+
+    #[derive(ptr_meta::Pointee)]
+    #[repr(C)]
+    struct CustomTag {
+        typ: TagTypeId,
+        size: u32,
+        string: [u8],
+    }
+
+    impl CustomTag {
+        fn string(&self) -> Result<&str, core::str::Utf8Error> {
+            Tag::get_dst_str_slice(&self.string)
+        }
+    }
+
+    impl TagTrait for CustomTag {
+        const ID: TagType = TagType::Custom(0x1337);
+
+        fn dst_size(base_tag: &Tag) -> usize {
+            assert!(base_tag.size as usize >= METADATA_SIZE);
+            base_tag.size as usize - METADATA_SIZE
+        }
+    }
+
+    #[test]
+    fn test_boxed_dst_tag() {
+        let content = "hallo";
+
+        let tag = BoxedDst::<CustomTag>::new(content.as_bytes());
+        assert_eq!(tag.typ, CustomTag::ID);
+        assert_eq!(tag.size as usize, METADATA_SIZE + content.len());
+        assert_eq!(tag.string(), Ok(content));
+    }
+}

+ 1 - 12
multiboot2/src/builder/information.rs

@@ -1,26 +1,15 @@
 //! Exports item [`InformationBuilder`].
+use crate::builder::{AsBytes, BoxedDst};
 use crate::{
     BasicMemoryInfoTag, BootInformationHeader, BootLoaderNameTag, CommandLineTag,
     EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag,
     EFISdt32Tag, EFISdt64Tag, ElfSectionsTag, EndTag, FramebufferTag, ImageLoadPhysAddrTag,
     MemoryMapTag, ModuleTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag, TagTrait, TagType,
 };
-
-use crate::builder::BoxedDst;
 use alloc::vec::Vec;
 use core::mem::size_of;
 use core::ops::Deref;
 
-/// Helper trait for all structs that need to be serialized that do not
-/// implement `TagTrait`.
-pub trait AsBytes: Sized {
-    fn as_bytes(&self) -> &[u8] {
-        let ptr = core::ptr::addr_of!(*self);
-        let size = core::mem::size_of::<Self>();
-        unsafe { core::slice::from_raw_parts(ptr.cast(), size) }
-    }
-}
-
 /// Holds the raw bytes of a boot information built with [`InformationBuilder`]
 /// on the heap. The bytes returned by [`BootInformationBytes::as_bytes`] are
 /// guaranteed to be properly aligned.

+ 10 - 140
multiboot2/src/builder/mod.rs

@@ -1,148 +1,18 @@
 //! Module for the builder-feature.
 
+mod boxed_dst;
 mod information;
 
-pub(crate) use information::AsBytes;
+// This must by public to support external people to create boxed DSTs.
+pub use boxed_dst::BoxedDst;
 pub use information::InformationBuilder;
 
-use alloc::alloc::alloc;
-use core::alloc::Layout;
-use core::marker::PhantomData;
-use core::mem::size_of;
-use core::ops::Deref;
-use core::ptr::NonNull;
-
-use crate::{Tag, TagTrait, TagTypeId};
-
-/// A helper type to create boxed DST, i.e., tags with a dynamic size for the
-/// builder. This is tricky in Rust. This type behaves similar to the regular
-/// `Box` type except that it ensure the same layout is used for the (explicit)
-/// allocation and the (implicit) deallocation of memory. Otherwise, I didn't
-/// found any way to figure out the right layout for a DST. Miri always reported
-/// issues that the deallocation used a wrong layout.
-///
-/// Technically, I'm certain this code is memory safe. But with this type, I
-/// also can convince miri that it is.
-#[derive(Debug, Eq)]
-pub struct BoxedDst<T: ?Sized> {
-    ptr: core::ptr::NonNull<T>,
-    layout: Layout,
-    // marker: I used this only as the regular Box impl also does it.
-    _marker: PhantomData<T>,
-}
-
-impl<T: TagTrait<Metadata = usize> + ?Sized> BoxedDst<T> {
-    /// Create a boxed tag with the given content.
-    ///
-    /// # Parameters
-    /// - `content` - All payload bytes of the DST tag without the tag type or
-    ///               the size. The memory is only read and can be discarded
-    ///               afterwards.
-    pub(crate) fn new(content: &[u8]) -> Self {
-        // Currently, I do not find a nice way of making this dynamic so that
-        // also miri is guaranteed to be happy. But it seems that 4 is fine
-        // here. I do have control over allocation and deallocation.
-        const ALIGN: usize = 4;
-
-        let tag_size = size_of::<TagTypeId>() + size_of::<u32>() + content.len();
-
-        // By using miri, I could figure out that there often are problems where
-        // miri thinks an allocation is smaller then necessary. Most probably
-        // due to not packed structs. Using packed structs however
-        // (especially with DSTs), is a crazy ass pain and unusable :/ Therefore,
-        // the best solution I can think of is to allocate a few byte more than
-        // necessary. I think that during runtime, everything works fine and
-        // that no memory issues are present.
-        let alloc_size = (tag_size + 7) & !7; // align to next 8 byte boundary
-        let layout = Layout::from_size_align(alloc_size, ALIGN).unwrap();
-        let ptr = unsafe { alloc(layout) };
-        assert!(!ptr.is_null());
-
-        // write tag content to memory
-        unsafe {
-            // write tag type
-            let ptrx = ptr.cast::<TagTypeId>();
-            ptrx.write(T::ID.into());
-
-            // write tag size
-            let ptrx = ptrx.add(1).cast::<u32>();
-            ptrx.write(tag_size as u32);
-
-            // write rest of content
-            let ptrx = ptrx.add(1).cast::<u8>();
-            let tag_content_slice = core::slice::from_raw_parts_mut(ptrx, content.len());
-            for (i, &byte) in content.iter().enumerate() {
-                tag_content_slice[i] = byte;
-            }
-        }
-
-        let base_tag = unsafe { &*ptr.cast::<Tag>() };
-        let raw: *mut T = ptr_meta::from_raw_parts_mut(ptr.cast(), T::dst_size(base_tag));
-
-        Self {
-            ptr: NonNull::new(raw).unwrap(),
-            layout,
-            _marker: PhantomData,
-        }
-    }
-}
-
-impl<T: ?Sized> Drop for BoxedDst<T> {
-    fn drop(&mut self) {
-        unsafe { alloc::alloc::dealloc(self.ptr.as_ptr().cast(), self.layout) }
-    }
-}
-
-impl<T: ?Sized> Deref for BoxedDst<T> {
-    type Target = T;
-    fn deref(&self) -> &Self::Target {
-        unsafe { self.ptr.as_ref() }
-    }
-}
-
-impl<T: ?Sized + PartialEq> PartialEq for BoxedDst<T> {
-    fn eq(&self, other: &Self) -> bool {
-        self.deref().eq(other.deref())
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::TagType;
-
-    const METADATA_SIZE: usize = 8;
-
-    #[derive(ptr_meta::Pointee)]
-    #[repr(C)]
-    struct CustomTag {
-        typ: TagTypeId,
-        size: u32,
-        string: [u8],
-    }
-
-    impl CustomTag {
-        fn string(&self) -> Result<&str, core::str::Utf8Error> {
-            Tag::get_dst_str_slice(&self.string)
-        }
-    }
-
-    impl TagTrait for CustomTag {
-        const ID: TagType = TagType::Custom(0x1337);
-
-        fn dst_size(base_tag: &Tag) -> usize {
-            assert!(base_tag.size as usize >= METADATA_SIZE);
-            base_tag.size as usize - METADATA_SIZE
-        }
-    }
-
-    #[test]
-    fn test_boxed_dst_tag() {
-        let content = "hallo";
-
-        let tag = BoxedDst::<CustomTag>::new(content.as_bytes());
-        assert_eq!(tag.typ, CustomTag::ID);
-        assert_eq!(tag.size as usize, METADATA_SIZE + content.len());
-        assert_eq!(tag.string(), Ok(content));
+/// Helper trait for all structs that need to be serialized that do not
+/// implement `TagTrait`.
+pub trait AsBytes: Sized {
+    fn as_bytes(&self) -> &[u8] {
+        let ptr = core::ptr::addr_of!(*self);
+        let size = core::mem::size_of::<Self>();
+        unsafe { core::slice::from_raw_parts(ptr.cast(), size) }
     }
 }