From 1ebc752e1ea5f18929703da6d251506e4e1f8817 Mon Sep 17 00:00:00 2001 From: Alex Olson <84994392+A0lson@users.noreply.github.com> Date: Mon, 6 Mar 2023 12:17:50 -0600 Subject: [PATCH 1/2] Fix two minor clippy issues --- multiboot2-header/src/builder/information_request.rs | 2 +- multiboot2-header/src/header.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/multiboot2-header/src/builder/information_request.rs b/multiboot2-header/src/builder/information_request.rs index 093e7c2a..528ec327 100644 --- a/multiboot2-header/src/builder/information_request.rs +++ b/multiboot2-header/src/builder/information_request.rs @@ -21,7 +21,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, diff --git a/multiboot2-header/src/header.rs b/multiboot2-header/src/header.rs index c84be710..7c74fe31 100644 --- a/multiboot2-header/src/header.rs +++ b/multiboot2-header/src/header.rs @@ -47,8 +47,7 @@ impl<'a> Multiboot2Header<'a> { assert_eq!( reference.header_magic(), MULTIBOOT2_HEADER_MAGIC, - "The Multiboot2 header must contain the MULTIBOOT2_HEADER_MAGIC={:x}", - MULTIBOOT2_HEADER_MAGIC + "The Multiboot2 header must contain the MULTIBOOT2_HEADER_MAGIC={MULTIBOOT2_HEADER_MAGIC:x}" ); assert!( reference.verify_checksum(), From ec1847842f383abcbcd0e6a4a8d6f7ce2d0d6827 Mon Sep 17 00:00:00 2001 From: Alex Olson <84994392+A0lson@users.noreply.github.com> Date: Fri, 24 Feb 2023 12:35:37 -0600 Subject: [PATCH 2/2] multiboot2: Fix EFI Memory Map 'last_area' calculation Correct EFI 'last_area' calculation so it correctly describes the greatest possible starting address of the last record. (Previously, iterating the EFI Memory map resulted in a superfluous entry as it ran over the next tag) This revision also cleans up the E820/EFI parsing to use 0-length arrays for the records and to faciliate a more natural 'sizeof' operation on the Multiboot2 tag. --- multiboot2/src/lib.rs | 14 ++++++++++++-- multiboot2/src/memory_map.rs | 17 +++++++++++------ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 83ad91fc..4546d695 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -1415,13 +1415,23 @@ mod tests { assert!(efi_mmap.is_none()); } + #[test] + /// Compile time test for `MemoryMapTag`. + fn e820_memory_map_tag_size() { + use super::MemoryMapTag; + unsafe { + // `MemoryMapTag` is 16 bytes without the 1st entry + core::mem::transmute::<[u8; 16], MemoryMapTag>([0u8; 16]); + } + } + #[test] /// Compile time test for `EFIMemoryMapTag`. fn efi_memory_map_tag_size() { use super::EFIMemoryMapTag; unsafe { - // `EFIMemoryMapTag` is 16 bytes + `EFIMemoryDesc` is 40 bytes. - core::mem::transmute::<[u8; 56], EFIMemoryMapTag>([0u8; 56]); + // `EFIMemoryMapTag` is 16 bytes without the 1st entry + core::mem::transmute::<[u8; 16], EFIMemoryMapTag>([0u8; 16]); } } } diff --git a/multiboot2/src/memory_map.rs b/multiboot2/src/memory_map.rs index c3056177..e8972947 100644 --- a/multiboot2/src/memory_map.rs +++ b/multiboot2/src/memory_map.rs @@ -18,7 +18,7 @@ pub struct MemoryMapTag { size: u32, entry_size: u32, entry_version: u32, - first_area: MemoryArea, + first_area: [MemoryArea; 0], } impl MemoryMapTag { @@ -31,10 +31,13 @@ impl MemoryMapTag { /// Return an iterator over all marked memory areas. pub fn all_memory_areas(&self) -> impl Iterator { let self_ptr = self as *const MemoryMapTag; - let start_area = (&self.first_area) as *const MemoryArea; + let start_area = self.first_area.as_ptr(); + MemoryAreaIter { current_area: start_area as u64, - last_area: (self_ptr as u64 + (self.size - self.entry_size) as u64), + // NOTE: `last_area` is only a bound, it doesn't necessarily point exactly to the last element + last_area: (self_ptr as u64 + + (self.size as u64 - core::mem::size_of::() as u64)), entry_size: self.entry_size, phantom: PhantomData, } @@ -127,7 +130,7 @@ pub struct EFIMemoryMapTag { size: u32, desc_size: u32, desc_version: u32, - first_desc: EFIMemoryDesc, + first_desc: [EFIMemoryDesc; 0], } impl EFIMemoryMapTag { @@ -137,10 +140,12 @@ impl EFIMemoryMapTag { /// available memory areas for tables and such. pub fn memory_areas(&self) -> EFIMemoryAreaIter { let self_ptr = self as *const EFIMemoryMapTag; - let start_area = (&self.first_desc) as *const EFIMemoryDesc; + let start_area = self.first_desc.as_ptr(); EFIMemoryAreaIter { current_area: start_area as u64, - last_area: (self_ptr as u64 + self.size as u64), + // NOTE: `last_area` is only a bound, it doesn't necessarily point exactly to the last element + last_area: (self_ptr as u64 + + (self.size as u64 - core::mem::size_of::() as u64)), entry_size: self.desc_size, phantom: PhantomData, }