diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 141c4ae8..22afdc1f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -20,7 +20,7 @@ jobs: name: build (msrv) uses: ./.github/workflows/_build-rust.yml with: - rust-version: 1.68.0 # MSRV + rust-version: 1.69.0 # MSRV do-style-check: false features: builder @@ -46,7 +46,7 @@ jobs: needs: build_msrv uses: ./.github/workflows/_build-rust.yml with: - rust-version: 1.68.0 # MSRV + rust-version: 1.69.0 # MSRV do-style-check: false rust-target: thumbv7em-none-eabihf features: builder @@ -103,7 +103,7 @@ jobs: needs: build_msrv uses: ./.github/workflows/_build-rust.yml with: - rust-version: 1.68.0 # MSRV + rust-version: 1.69.0 # MSRV do-style-check: true do-test: false features: builder diff --git a/Cargo.lock b/Cargo.lock index 43ab84a7..d9d54e11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -40,7 +40,7 @@ dependencies = [ [[package]] name = "multiboot2" -version = "0.18.1" +version = "0.19.0" dependencies = [ "bitflags", "derive_more", diff --git a/integration-test/bins/Cargo.lock b/integration-test/bins/Cargo.lock index 8bc30cb4..ff272cbd 100644 --- a/integration-test/bins/Cargo.lock +++ b/integration-test/bins/Cargo.lock @@ -109,7 +109,7 @@ dependencies = [ [[package]] name = "multiboot2" -version = "0.18.1" +version = "0.19.0" dependencies = [ "bitflags 2.4.0", "derive_more", @@ -135,7 +135,7 @@ dependencies = [ "good_memory_allocator", "log", "multiboot", - "multiboot2 0.18.1", + "multiboot2 0.19.0", "multiboot2-header", "util", ] @@ -147,7 +147,7 @@ dependencies = [ "anyhow", "good_memory_allocator", "log", - "multiboot2 0.18.1", + "multiboot2 0.19.0", "util", "x86", ] diff --git a/multiboot2/Cargo.toml b/multiboot2/Cargo.toml index 2b4ea5fe..3a31f84b 100644 --- a/multiboot2/Cargo.toml +++ b/multiboot2/Cargo.toml @@ -6,7 +6,7 @@ Multiboot2-compliant bootloaders, such as GRUB. It supports all tags from the specification including full support for the sections of ELF files. This library is `no_std` and can be used in a Multiboot2-kernel. """ -version = "0.18.1" +version = "0.19.0" authors = [ "Philipp Oppermann ", "Calvin Lee ", diff --git a/multiboot2/Changelog.md b/multiboot2/Changelog.md index 4565f755..20c1124b 100644 --- a/multiboot2/Changelog.md +++ b/multiboot2/Changelog.md @@ -1,6 +1,15 @@ # CHANGELOG for crate `multiboot2` -## 0.19.0 (2023-07-14) +## 0.19.0 (2023-09-XX) +- **BREAKING** MSRV is 1.69.0 +- **BREAKING** `Tag::get_dst_str_slice` renamed to + `Tag::parse_slice_as_string` and now returns `Result<&str, StringError>` +- **BREAKING** `BootLoaderNameTag::name` now returns `Result<&str, StringError>` +- **BREAKING** `CommandLineTag::cmdline` now returns `Result<&str, StringError>` +- **BREAKING** `ModuleTag::cmdline` now returns `Result<&str, StringError>` +- Introduced new enum type `StringError` +- Additionally, a bug was fixed in `parse_slice_as_string` which now parses + multiboot2 strings as expected: as null-terminated UTF-8 strings. - `InformationBuilder` now also allows to add custom tags. The new public method `add_tag` was introduced for that. diff --git a/multiboot2/README.md b/multiboot2/README.md index c6e8552a..ea616127 100644 --- a/multiboot2/README.md +++ b/multiboot2/README.md @@ -38,7 +38,7 @@ other fields | variable All tags and the mbi itself are 8-byte aligned. The last tag must be the _end tag_, which is a tag of type `0` and size `8`. ## MSRV -The MSRV is 1.68.0 stable. +The MSRV is 1.69.0 stable. ## License & Contribution diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index a7505c4c..93b12e0d 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -1,9 +1,9 @@ //! Module for [`BootLoaderNameTag`]. +use crate::tag::StringError; use crate::{Tag, TagTrait, TagType, TagTypeId}; use core::fmt::{Debug, Formatter}; use core::mem::size_of; -use core::str::Utf8Error; #[cfg(feature = "builder")] use {crate::builder::BoxedDst, alloc::vec::Vec}; @@ -47,8 +47,8 @@ impl BootLoaderNameTag { /// assert_eq!(Ok("GRUB 2.02~beta3-5"), tag.name()); /// } /// ``` - pub fn name(&self) -> Result<&str, Utf8Error> { - Tag::get_dst_str_slice(&self.name) + pub fn name(&self) -> Result<&str, StringError> { + Tag::parse_slice_as_string(&self.name) } } diff --git a/multiboot2/src/builder/boxed_dst.rs b/multiboot2/src/builder/boxed_dst.rs index f75da1fb..9106457f 100644 --- a/multiboot2/src/builder/boxed_dst.rs +++ b/multiboot2/src/builder/boxed_dst.rs @@ -103,6 +103,7 @@ impl PartialEq for BoxedDst { #[cfg(test)] mod tests { use super::*; + use crate::tag::StringError; use crate::TagType; const METADATA_SIZE: usize = 8; @@ -116,8 +117,8 @@ mod tests { } impl CustomTag { - fn string(&self) -> Result<&str, core::str::Utf8Error> { - Tag::get_dst_str_slice(&self.string) + fn string(&self) -> Result<&str, StringError> { + Tag::parse_slice_as_string(&self.string) } } @@ -132,11 +133,12 @@ mod tests { #[test] fn test_boxed_dst_tag() { - let content = "hallo"; + let content = b"hallo\0"; + let content_rust_str = "hallo"; - let tag = BoxedDst::::new(content.as_bytes()); + let tag = BoxedDst::::new(content); assert_eq!(tag.typ, CustomTag::ID); assert_eq!(tag.size as usize, METADATA_SIZE + content.len()); - assert_eq!(tag.string(), Ok(content)); + assert_eq!(tag.string(), Ok(content_rust_str)); } } diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index 2f20227a..e2b168f8 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -2,10 +2,10 @@ use crate::{Tag, TagTrait, TagType, TagTypeId}; +use crate::tag::StringError; use core::fmt::{Debug, Formatter}; use core::mem; use core::str; - #[cfg(feature = "builder")] use {crate::builder::BoxedDst, alloc::vec::Vec}; @@ -55,8 +55,8 @@ impl CommandLineTag { /// assert_eq!(Ok("/bootarg"), command_line); /// } /// ``` - pub fn cmdline(&self) -> Result<&str, str::Utf8Error> { - Tag::get_dst_str_slice(&self.cmdline) + pub fn cmdline(&self) -> Result<&str, StringError> { + Tag::parse_slice_as_string(&self.cmdline) } } diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index c63c13f6..4f8b05cf 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -33,7 +33,7 @@ //! ``` //! //! ## MSRV -//! The MSRV is 1.68.0 stable. +//! The MSRV is 1.69.0 stable. #[cfg(feature = "builder")] extern crate alloc; @@ -84,7 +84,7 @@ pub use module::{ModuleIter, ModuleTag}; pub use ptr_meta::Pointee; pub use rsdp::{RsdpV1Tag, RsdpV2Tag}; pub use smbios::SmbiosTag; -pub use tag::Tag; +pub use tag::{StringError, Tag}; pub use tag_trait::TagTrait; pub use tag_type::{TagType, TagTypeId}; pub use vbe_info::{ @@ -447,7 +447,7 @@ impl<'a> BootInformation<'a> { /// /// impl CustomTag { /// fn name(&self) -> Result<&str, Utf8Error> { - /// Tag::get_dst_str_slice(&self.name) + /// Tag::parse_slice_as_string(&self.name) /// } /// } /// let mbi_ptr = 0xdeadbeef as *const BootInformationHeader; @@ -524,7 +524,7 @@ impl fmt::Debug for BootInformation<'_> { mod tests { use super::*; use crate::memory_map::MemoryAreaType; - use core::str::Utf8Error; + use crate::tag::StringError; /// Compile time test to check if the boot information is Send and Sync. /// This test is relevant to give library users flexebility in passing the @@ -542,7 +542,6 @@ mod tests { 8, 0, 0, 0, // end tag size ]); let ptr = bytes.0.as_ptr(); - let addr = ptr as usize; let bi = unsafe { BootInformation::load(ptr.cast()) }; let bi = bi.unwrap(); @@ -1604,8 +1603,8 @@ mod tests { } impl CustomTag { - fn name(&self) -> Result<&str, Utf8Error> { - Tag::get_dst_str_slice(&self.name) + fn name(&self) -> Result<&str, StringError> { + Tag::parse_slice_as_string(&self.name) } } diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index 6392331e..84c8f2b2 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -1,10 +1,9 @@ //! Module for [`ModuleTag`]. +use crate::tag::StringError; use crate::{Tag, TagIter, TagTrait, TagType, TagTypeId}; use core::fmt::{Debug, Formatter}; use core::mem::size_of; -use core::str::Utf8Error; - #[cfg(feature = "builder")] use {crate::builder::BoxedDst, alloc::vec::Vec}; @@ -48,8 +47,8 @@ impl ModuleTag { /// contains `"module2 /some_boot_module --test cmdline-option"`. /// /// If the function returns `Err` then perhaps the memory is invalid. - pub fn cmdline(&self) -> Result<&str, Utf8Error> { - Tag::get_dst_str_slice(&self.cmdline) + pub fn cmdline(&self) -> Result<&str, StringError> { + Tag::parse_slice_as_string(&self.cmdline) } /// Start address of the module. diff --git a/multiboot2/src/tag.rs b/multiboot2/src/tag.rs index c7860111..0002d59f 100644 --- a/multiboot2/src/tag.rs +++ b/multiboot2/src/tag.rs @@ -4,10 +4,36 @@ use crate::{TagTrait, TagType, TagTypeId}; use core::fmt; -use core::fmt::{Debug, Formatter}; +use core::fmt::{Debug, Display, Formatter}; use core::marker::PhantomData; use core::str::Utf8Error; +/// Error type describing failures when parsing the string from a tag. +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum StringError { + /// There is no terminating NUL character, although the specification + /// requires one. + MissingNul(core::ffi::FromBytesUntilNulError), + /// The sequence until the first NUL character is not valid UTF-8. + Utf8(Utf8Error), +} + +impl Display for StringError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +#[cfg(feature = "unstable")] +impl core::error::Error for StringError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + StringError::MissingNul(e) => Some(e), + StringError::Utf8(e) => Some(e), + } + } +} + /// Common base structure for all tags that can be passed via the Multiboot2 /// Information Structure (MBI) to a Multiboot2 payload/program/kernel. /// @@ -38,22 +64,12 @@ impl Tag { 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> { - // Determine length of string before first NUL character - - let length = match bytes.iter().position(|ch| (*ch) == 0x00) { - // Unterminated string case: - None => bytes.len(), - - // Terminated case: - Some(idx) => idx, - }; - // Convert to Rust string: - core::str::from_utf8(&bytes[..length]) + /// Parses the provided byte sequence as Multiboot string, which maps to a + /// [`str`]. + pub fn parse_slice_as_string(bytes: &[u8]) -> Result<&str, StringError> { + let cstr = core::ffi::CStr::from_bytes_until_nul(bytes).map_err(StringError::MissingNul)?; + + cstr.to_str().map_err(StringError::Utf8) } } @@ -128,21 +144,28 @@ 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(b"foobar")); - // test that the null bytes is not included in the string slice - assert_eq!(Ok("foobar"), Tag::get_dst_str_slice(b"foobar\0")); - // test that C-style null string termination works as expected - assert_eq!(Ok("foo"), Tag::get_dst_str_slice(b"foo\0bar")); - // test invalid utf8 + fn parse_slice_as_string() { + // empty slice is invalid + assert!(matches!( + Tag::parse_slice_as_string(&[]), + Err(StringError::MissingNul(_)) + )); + // empty string is fine + assert_eq!(Tag::parse_slice_as_string(&[0x00]), Ok("")); + // reject invalid utf8 + assert!(matches!( + Tag::parse_slice_as_string(&[0xff, 0x00]), + Err(StringError::Utf8(_)) + )); + // reject missing null assert!(matches!( - Tag::get_dst_str_slice(&[0xff, 0xff]), - Err(Utf8Error { .. }) + Tag::parse_slice_as_string(b"hello"), + Err(StringError::MissingNul(_)) )); + // must not include final null + assert_eq!(Tag::parse_slice_as_string(b"hello\0"), Ok("hello")); + assert_eq!(Tag::parse_slice_as_string(b"hello\0\0"), Ok("hello")); + // must skip everytihng after first null + assert_eq!(Tag::parse_slice_as_string(b"hello\0foo"), Ok("hello")); } }