From e2e5b869b2eaca205361c955111b42fe1598df7a Mon Sep 17 00:00:00 2001 From: John Eckersberg Date: Tue, 11 Nov 2025 09:36:22 -0500 Subject: [PATCH 1/6] kernel_cmdline: Add some more derives for Cmdline Prep so we can parse these directly via clap Signed-off-by: John Eckersberg --- Cargo.lock | 1 + crates/kernel_cmdline/Cargo.toml | 1 + crates/kernel_cmdline/src/bytes.rs | 3 ++- crates/kernel_cmdline/src/utf8.rs | 3 ++- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c4e0db96..876a73d08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -212,6 +212,7 @@ name = "bootc-kernel-cmdline" version = "0.0.0" dependencies = [ "anyhow", + "serde", "similar-asserts", "static_assertions", ] diff --git a/crates/kernel_cmdline/Cargo.toml b/crates/kernel_cmdline/Cargo.toml index 1e51ad034..548b1f82f 100644 --- a/crates/kernel_cmdline/Cargo.toml +++ b/crates/kernel_cmdline/Cargo.toml @@ -9,6 +9,7 @@ repository = "https://github.com/bootc-dev/bootc" [dependencies] # Workspace dependencies anyhow = { workspace = true } +serde = { workspace = true, features = ["derive"] } [dev-dependencies] similar-asserts = { workspace = true } diff --git a/crates/kernel_cmdline/src/bytes.rs b/crates/kernel_cmdline/src/bytes.rs index d96bfa51b..63c5838e9 100644 --- a/crates/kernel_cmdline/src/bytes.rs +++ b/crates/kernel_cmdline/src/bytes.rs @@ -9,13 +9,14 @@ use std::ops::Deref; use crate::{utf8, Action}; use anyhow::Result; +use serde::{Deserialize, Serialize}; /// A parsed kernel command line. /// /// Wraps the raw command line bytes and provides methods for parsing and iterating /// over individual parameters. Uses copy-on-write semantics to avoid unnecessary /// allocations when working with borrowed data. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct Cmdline<'a>(Cow<'a, [u8]>); /// An owned Cmdline. Alias for `Cmdline<'static>`. diff --git a/crates/kernel_cmdline/src/utf8.rs b/crates/kernel_cmdline/src/utf8.rs index 9af6a3c76..6b3f3b7cf 100644 --- a/crates/kernel_cmdline/src/utf8.rs +++ b/crates/kernel_cmdline/src/utf8.rs @@ -8,13 +8,14 @@ use std::ops::Deref; use crate::{bytes, Action}; use anyhow::Result; +use serde::{Deserialize, Serialize}; /// A parsed UTF-8 kernel command line. /// /// Wraps the raw command line bytes and provides methods for parsing and iterating /// over individual parameters. Uses copy-on-write semantics to avoid unnecessary /// allocations when working with borrowed data. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct Cmdline<'a>(bytes::Cmdline<'a>); /// An owned `Cmdline`. Alias for `Cmdline<'static>`. From 1f1b98622630b8acd6bc1f56e3c3510f8ec5f599 Mon Sep 17 00:00:00 2001 From: John Eckersberg Date: Tue, 11 Nov 2025 09:37:55 -0500 Subject: [PATCH 2/6] install: Hoist InstallResetOpts kargs parsing up into clap Signed-off-by: John Eckersberg --- crates/lib/src/install.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index dec2e0fc3..a491152c8 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -27,7 +27,7 @@ use std::time::Duration; use aleph::InstallAleph; use anyhow::{anyhow, ensure, Context, Result}; -use bootc_kernel_cmdline::utf8::{Cmdline, Parameter}; +use bootc_kernel_cmdline::utf8::{Cmdline, CmdlineOwned}; use bootc_utils::CommandRunExt; use camino::Utf8Path; use camino::Utf8PathBuf; @@ -434,7 +434,7 @@ pub(crate) struct InstallResetOpts { /// /// Example: --karg=nosmt --karg=console=ttyS0,114800n8 #[clap(long)] - karg: Option>, + karg: Option>, } /// Global state captured from the container. @@ -2335,15 +2335,10 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { } // Extend with user-provided kargs - if let Some(user_kargs) = opts.karg { - let user_kargs = user_kargs - .iter() - .map(|arg| { - Parameter::parse(arg).ok_or_else(|| anyhow!("Unable to parse parameter: {arg}")) - }) - .collect::>>()?; - - kargs.extend(user_kargs); + if let Some(user_kargs) = opts.karg.as_ref() { + for karg in user_kargs { + kargs.extend(karg); + } } let from = MergeState::Reset { From c82523d0316166c26d4ee57fc4e1831f0814c27e Mon Sep 17 00:00:00 2001 From: John Eckersberg Date: Wed, 12 Nov 2025 14:04:13 -0500 Subject: [PATCH 3/6] kernel_cmdline: Standardize Deref/AsRef impls across types Implement generic AsRef in terms of Deref as suggested in: https://doc.rust-lang.org/std/convert/trait.AsRef.html#generic-implementations I'm not 100% sold on this since in some case (like the change to the tests here) you have to end up doing `&*` coercion to get the right type, but this at least makes everything consistent. Would be a bit nicer when/if str_as_str[1] stabilizes, at least for the most common UTF-8 case. [1] https://github.com/rust-lang/rust/issues/130366 Signed-off-by: John Eckersberg --- crates/kernel_cmdline/src/bytes.rs | 34 +++++++++++++++++++++--------- crates/kernel_cmdline/src/utf8.rs | 29 ++++++++++++++++++------- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/crates/kernel_cmdline/src/bytes.rs b/crates/kernel_cmdline/src/bytes.rs index 63c5838e9..b6c1785c9 100644 --- a/crates/kernel_cmdline/src/bytes.rs +++ b/crates/kernel_cmdline/src/bytes.rs @@ -39,6 +39,16 @@ impl Deref for Cmdline<'_> { } } +impl<'a, T> AsRef for Cmdline<'a> +where + T: ?Sized, + as Deref>::Target: AsRef, +{ + fn as_ref(&self) -> &T { + self.deref().as_ref() + } +} + impl From> for CmdlineOwned { /// Creates a new `Cmdline` from an owned `Vec`. fn from(input: Vec) -> Self { @@ -340,12 +350,6 @@ impl<'a> Cmdline<'a> { } } -impl<'a> AsRef<[u8]> for Cmdline<'a> { - fn as_ref(&self) -> &[u8] { - &self.0 - } -} - impl<'a> IntoIterator for &'a Cmdline<'a> { type Item = Parameter<'a>; type IntoIter = CmdlineIter<'a>; @@ -376,10 +380,10 @@ impl<'a, 'other> Extend> for Cmdline<'a> { #[derive(Clone, Debug, Eq)] pub struct ParameterKey<'a>(pub(crate) &'a [u8]); -impl<'a> Deref for ParameterKey<'a> { +impl Deref for ParameterKey<'_> { type Target = [u8]; - fn deref(&self) -> &'a Self::Target { + fn deref(&self) -> &Self::Target { self.0 } } @@ -509,14 +513,24 @@ impl<'a> PartialEq for Parameter<'a> { } } -impl<'a> std::ops::Deref for Parameter<'a> { +impl Deref for Parameter<'_> { type Target = [u8]; - fn deref(&self) -> &'a Self::Target { + fn deref(&self) -> &Self::Target { self.parameter } } +impl<'a, T> AsRef for Parameter<'a> +where + T: ?Sized, + as Deref>::Target: AsRef, +{ + fn as_ref(&self) -> &T { + self.deref().as_ref() + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/kernel_cmdline/src/utf8.rs b/crates/kernel_cmdline/src/utf8.rs index 6b3f3b7cf..49774a64a 100644 --- a/crates/kernel_cmdline/src/utf8.rs +++ b/crates/kernel_cmdline/src/utf8.rs @@ -224,10 +224,13 @@ impl Deref for Cmdline<'_> { } } -impl<'a> AsRef for Cmdline<'a> { - fn as_ref(&self) -> &str { - str::from_utf8(self.0.as_ref()) - .expect("We only construct the underlying bytes from valid UTF-8") +impl<'a, T> AsRef for Cmdline<'a> +where + T: ?Sized, + as Deref>::Target: AsRef, +{ + fn as_ref(&self) -> &T { + self.deref().as_ref() } } @@ -267,7 +270,7 @@ impl<'a, 'other> Extend> for Cmdline<'a> { #[derive(Clone, Debug, Eq)] pub struct ParameterKey<'a>(bytes::ParameterKey<'a>); -impl<'a> Deref for ParameterKey<'a> { +impl Deref for ParameterKey<'_> { type Target = str; fn deref(&self) -> &Self::Target { @@ -381,7 +384,7 @@ impl<'a> std::fmt::Display for Parameter<'a> { } } -impl<'a> Deref for Parameter<'a> { +impl Deref for Parameter<'_> { type Target = str; fn deref(&self) -> &Self::Target { @@ -391,6 +394,16 @@ impl<'a> Deref for Parameter<'a> { } } +impl<'a, T> AsRef for Parameter<'a> +where + T: ?Sized, + as Deref>::Target: AsRef, +{ + fn as_ref(&self) -> &T { + self.deref().as_ref() + } +} + impl<'a> PartialEq for Parameter<'a> { fn eq(&self, other: &Self) -> bool { self.0 == other.0 @@ -769,7 +782,7 @@ mod tests { fn test_add_empty_cmdline() { let mut kargs = Cmdline::from(""); assert!(matches!(kargs.add(¶m("foo")), Action::Added)); - assert_eq!(kargs.as_ref(), "foo"); + assert_eq!(&*kargs, "foo"); } #[test] @@ -809,7 +822,7 @@ mod tests { fn test_add_or_modify_empty_cmdline() { let mut kargs = Cmdline::from(""); assert!(matches!(kargs.add_or_modify(¶m("foo")), Action::Added)); - assert_eq!(kargs.as_ref(), "foo"); + assert_eq!(&*kargs, "foo"); } #[test] From 1a613caefc33b91f08ae50d3a219604dc2a4af6f Mon Sep 17 00:00:00 2001 From: John Eckersberg Date: Thu, 13 Nov 2025 09:33:10 -0500 Subject: [PATCH 4/6] Use Cmdline for kargs fields in InstallConfigOpts and RootSetup Plus all of the various places this trickles down. Signed-off-by: John Eckersberg --- crates/lib/src/bootc_composefs/boot.rs | 25 +++++++------ crates/lib/src/install.rs | 49 +++++++++++++++----------- crates/lib/src/install/baseline.rs | 32 +++++++++++++---- crates/lib/src/parsers/bls_config.rs | 10 +++--- 4 files changed, 73 insertions(+), 43 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index d00908598..fd4c71981 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -5,6 +5,7 @@ use std::path::Path; use anyhow::{anyhow, Context, Result}; use bootc_blockdev::find_parent_devices; +use bootc_kernel_cmdline::utf8::Cmdline; use bootc_mount::inspect_filesystem_of_dir; use bootc_mount::tempmount::TempMount; use camino::{Utf8Path, Utf8PathBuf}; @@ -380,13 +381,17 @@ pub(crate) fn setup_composefs_bls_boot( let (root_path, esp_device, cmdline_refs, fs, bootloader) = match setup_type { BootSetupType::Setup((root_setup, state, fs)) => { // root_setup.kargs has [root=UUID=, "rw"] - let mut cmdline_options = String::from(root_setup.kargs.join(" ")); + let mut cmdline_options = Cmdline::new(); - if state.composefs_options.insecure { - cmdline_options.push_str(&format!(" {COMPOSEFS_CMDLINE}=?{id_hex}")); + cmdline_options.extend(&root_setup.kargs); + + let composefs_cmdline = if state.composefs_options.insecure { + format!("{COMPOSEFS_CMDLINE}=?{id_hex}") } else { - cmdline_options.push_str(&format!(" {COMPOSEFS_CMDLINE}={id_hex}")); - } + format!("{COMPOSEFS_CMDLINE}={id_hex}") + }; + + cmdline_options.extend(&Cmdline::from(&composefs_cmdline)); // Locate ESP partition device let esp_part = esp_in(&root_setup.device_info)?; @@ -407,12 +412,10 @@ pub(crate) fn setup_composefs_bls_boot( ( Utf8PathBuf::from("/sysroot"), get_esp_partition(&sysroot_parent)?.0, - [ - format!("root=UUID={}", this_arch_root()), - RW_KARG.to_string(), - format!("{COMPOSEFS_CMDLINE}={id_hex}"), - ] - .join(" "), + Cmdline::from(format!( + "root=UUID={} {RW_KARG} {COMPOSEFS_CMDLINE}={id_hex}", + this_arch_root() + )), fs, bootloader, ) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index a491152c8..66c62f091 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -196,7 +196,7 @@ pub(crate) struct InstallConfigOpts { /// /// Example: --karg=nosmt --karg=console=ttyS0,115200n8 #[clap(long)] - pub(crate) karg: Option>, + pub(crate) karg: Option>, /// The path to an `authorized_keys` that will be injected into the `root` account. /// @@ -920,7 +920,6 @@ async fn install_container( merged_ostree_root.downcast_ref().unwrap(), std::env::consts::ARCH, )?; - let kargsd = kargsd.iter_str().collect::>(); // If the target uses aboot, then we need to set that bootloader in the ostree // config before deploying the commit @@ -942,28 +941,36 @@ async fn install_container( } // Keep this in sync with install/completion.rs for the Anaconda fixups - let install_config_kargs = state - .install_config - .as_ref() - .and_then(|c| c.kargs.as_ref()) - .into_iter() - .flatten() - .map(|s| s.as_str()); + let install_config_kargs = state.install_config.as_ref().and_then(|c| c.kargs.as_ref()); + // Final kargs, in order: // - root filesystem kargs // - install config kargs // - kargs.d from container image // - args specified on the CLI - let kargs = root_setup - .kargs - .iter() - .map(|v| v.as_str()) - .chain(install_config_kargs) - .chain(kargsd) - .chain(state.config_opts.karg.iter().flatten().map(|v| v.as_str())) - .collect::>(); + let mut kargs = Cmdline::new(); + + kargs.extend(&root_setup.kargs); + + if let Some(install_config_kargs) = install_config_kargs { + for karg in install_config_kargs { + kargs.extend(&Cmdline::from(karg.as_str())); + } + } + + kargs.extend(&kargsd); + + if let Some(cli_kargs) = state.config_opts.karg.as_ref() { + for karg in cli_kargs { + kargs.extend(karg); + } + } + + // Finally map into &[&str] for ostree_container + let kargs_strs: Vec<&str> = kargs.iter_str().collect(); + let mut options = ostree_container::deploy::DeployOpts::default(); - options.kargs = Some(kargs.as_slice()); + options.kargs = Some(kargs_strs.as_slice()); options.target_imgref = Some(&state.target_imgref); options.proxy_cfg = proxy_cfg; options.skip_completion = true; // Must be set to avoid recursion! @@ -1077,7 +1084,7 @@ pub(crate) struct RootSetup { /// True if we should skip finalizing skip_finalize: bool, boot: Option, - pub(crate) kargs: Vec, + pub(crate) kargs: CmdlineOwned, } fn require_boot_uuid(spec: &MountSpec) -> Result<&str> { @@ -1631,7 +1638,7 @@ async fn install_to_filesystem_impl( cleanup: Cleanup, ) -> Result<()> { if matches!(state.selinux_state, SELinuxFinalState::ForceTargetDisabled) { - rootfs.kargs.push("selinux=0".to_string()); + rootfs.kargs.extend(&Cmdline::from("selinux=0")); } // Drop exclusive ownership since we're done with mutation let rootfs = &*rootfs; @@ -2167,6 +2174,8 @@ pub(crate) async fn install_to_filesystem( kargs.push(bootarg); } + let kargs = Cmdline::from(kargs.join(" ")); + let skip_finalize = matches!(fsopts.replace, Some(ReplaceMode::Alongside)) || fsopts.skip_finalize; let mut rootfs = RootSetup { diff --git a/crates/lib/src/install/baseline.rs b/crates/lib/src/install/baseline.rs index b20aff31d..cff80ce7c 100644 --- a/crates/lib/src/install/baseline.rs +++ b/crates/lib/src/install/baseline.rs @@ -30,6 +30,7 @@ use super::State; use super::RUN_BOOTC; use super::RW_KARG; use crate::task::Task; +use bootc_kernel_cmdline::utf8::Cmdline; #[cfg(feature = "install-to-disk")] use bootc_mount::is_mounted_in_pid1_mountns; @@ -424,13 +425,30 @@ pub(crate) fn install_create_rootfs( fstype: MountSpec::AUTO.into(), options: Some("ro".into()), }); - let kargs = root_blockdev_kargs - .into_iter() - .flatten() - .chain([rootarg, RW_KARG.to_string()].into_iter()) - .chain(bootarg) - .chain(state.config_opts.karg.clone().into_iter().flatten()) - .collect::>(); + + let mut kargs = Cmdline::new(); + + // Add root blockdev kargs (e.g., LUKS parameters) + if let Some(root_blockdev_kargs) = root_blockdev_kargs { + for karg in root_blockdev_kargs { + kargs.extend(&Cmdline::from(karg.as_str())); + } + } + + // Add root= and rw argument + kargs.extend(&Cmdline::from(format!("{rootarg} {RW_KARG}"))); + + // Add boot= argument if present + if let Some(bootarg) = bootarg { + kargs.extend(&Cmdline::from(bootarg.as_str())); + } + + // Add CLI kargs + if let Some(cli_kargs) = state.config_opts.karg.as_ref() { + for karg in cli_kargs { + kargs.extend(karg); + } + } bootc_mount::mount(&rootdev, &physical_root_path)?; let target_rootfs = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?; diff --git a/crates/lib/src/parsers/bls_config.rs b/crates/lib/src/parsers/bls_config.rs index 5f96fe647..606b990c7 100644 --- a/crates/lib/src/parsers/bls_config.rs +++ b/crates/lib/src/parsers/bls_config.rs @@ -5,7 +5,7 @@ #![allow(dead_code)] use anyhow::{anyhow, Result}; -use bootc_kernel_cmdline::utf8::Cmdline; +use bootc_kernel_cmdline::utf8::{Cmdline, CmdlineOwned}; use camino::Utf8PathBuf; use composefs_boot::bootloader::EFI_EXT; use core::fmt; @@ -15,7 +15,7 @@ use uapi_version::Version; use crate::composefs_consts::COMPOSEFS_CMDLINE; -#[derive(Debug, PartialEq, PartialOrd, Eq, Default)] +#[derive(Debug, PartialEq, Eq, Default)] pub enum BLSConfigType { EFI { /// The path to the EFI binary, usually a UKI @@ -27,7 +27,7 @@ pub enum BLSConfigType { /// The paths to the initrd images. initrd: Vec, /// Kernel command line options. - options: Option, + options: Option, }, #[default] Unknown, @@ -229,7 +229,7 @@ pub(crate) fn parse_bls_config(input: &str) -> Result { "version" => version = Some(value), "linux" => linux = Some(Utf8PathBuf::from(value)), "initrd" => initrd.push(Utf8PathBuf::from(value)), - "options" => options = Some(value), + "options" => options = Some(CmdlineOwned::from(value)), "machine-id" => machine_id = Some(value), "sort-key" => sort_key = Some(value), "efi" => efi = Some(Utf8PathBuf::from(value)), @@ -301,7 +301,7 @@ mod tests { assert_eq!(config.version, "2"); assert_eq!(linux, "/boot/7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6/vmlinuz-5.14.10"); assert_eq!(initrd, vec!["/boot/7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6/initramfs-5.14.10.img"]); - assert_eq!(options, Some("root=UUID=abc123 rw composefs=7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6".to_string())); + assert_eq!(&*options.unwrap(), "root=UUID=abc123 rw composefs=7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6"); assert_eq!(config.extra.get("custom1"), Some(&"value1".to_string())); assert_eq!(config.extra.get("custom2"), Some(&"value2".to_string())); From 44bdb6738060cab8d66712e3e358d70afd7539a9 Mon Sep 17 00:00:00 2001 From: John Eckersberg Date: Thu, 13 Nov 2025 14:15:45 -0500 Subject: [PATCH 5/6] kernel_cmdline: impl PartialOrd/Ord/PartialEq/Eq for Parameter/ParameterKey We need to be able to check equality on Cmdline by comparing sorted parameters. Signed-off-by: John Eckersberg --- crates/kernel_cmdline/src/bytes.rs | 59 +++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/crates/kernel_cmdline/src/bytes.rs b/crates/kernel_cmdline/src/bytes.rs index b6c1785c9..45f6a6e30 100644 --- a/crates/kernel_cmdline/src/bytes.rs +++ b/crates/kernel_cmdline/src/bytes.rs @@ -4,6 +4,7 @@ //! arguments, supporting both key-only switches and key-value pairs with proper quote handling. use std::borrow::Cow; +use std::cmp::Ordering; use std::ops::Deref; use crate::{utf8, Action}; @@ -377,7 +378,7 @@ impl<'a, 'other> Extend> for Cmdline<'a> { /// A single kernel command line parameter key /// /// Handles quoted values and treats dashes and underscores in keys as equivalent. -#[derive(Clone, Debug, Eq)] +#[derive(Clone, Debug)] pub struct ParameterKey<'a>(pub(crate) &'a [u8]); impl Deref for ParameterKey<'_> { @@ -404,32 +405,42 @@ impl<'a, T: AsRef<[u8]> + ?Sized> From<&'a T> for ParameterKey<'a> { } } +impl ParameterKey<'_> { + /// Returns an iterator over the canonicalized bytes of the + /// parameter, with dashes turned into underscores. + fn iter(&self) -> impl Iterator + use<'_> { + self.0 + .iter() + .map(|&c: &u8| if c == b'-' { b'_' } else { c }) + } +} + impl PartialEq for ParameterKey<'_> { /// Compares two parameter keys for equality. /// /// Keys are compared with dashes and underscores treated as equivalent. /// This comparison is case-sensitive. fn eq(&self, other: &Self) -> bool { - let dedashed = |&c: &u8| { - if c == b'-' { - b'_' - } else { - c - } - }; + self.iter().eq(other.iter()) + } +} + +impl Eq for ParameterKey<'_> {} - // We can't just zip() because leading substrings will match - // - // For example, "foo" == "foobar" since the zipped iterator - // only compares the first three chars. - let our_iter = self.0.iter().map(dedashed); - let other_iter = other.0.iter().map(dedashed); - our_iter.eq(other_iter) +impl Ord for ParameterKey<'_> { + fn cmp(&self, other: &Self) -> Ordering { + self.iter().cmp(other.iter()) + } +} + +impl PartialOrd for ParameterKey<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) } } /// A single kernel command line parameter. -#[derive(Clone, Debug, Eq)] +#[derive(Clone, Debug)] pub struct Parameter<'a> { /// The full original value parameter: &'a [u8], @@ -506,13 +517,27 @@ impl<'a> Parameter<'a> { } } -impl<'a> PartialEq for Parameter<'a> { +impl PartialEq for Parameter<'_> { fn eq(&self, other: &Self) -> bool { // Note we don't compare parameter because we want hyphen-dash insensitivity for the key self.key == other.key && self.value == other.value } } +impl Eq for Parameter<'_> {} + +impl Ord for Parameter<'_> { + fn cmp(&self, other: &Self) -> Ordering { + self.key.cmp(&other.key).then(self.value.cmp(&other.value)) + } +} + +impl PartialOrd for Parameter<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl Deref for Parameter<'_> { type Target = [u8]; From ec3895cd382c089b043d53d40904f154e5557e98 Mon Sep 17 00:00:00 2001 From: John Eckersberg Date: Thu, 13 Nov 2025 11:39:01 -0500 Subject: [PATCH 6/6] kernel_cmdline: Manually impl semantically-correct equivalence for Cmdline Signed-off-by: John Eckersberg --- crates/kernel_cmdline/src/bytes.rs | 35 +++++++++++++++++++++++++++++- crates/kernel_cmdline/src/utf8.rs | 20 +++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/kernel_cmdline/src/bytes.rs b/crates/kernel_cmdline/src/bytes.rs index 45f6a6e30..ac6e70f66 100644 --- a/crates/kernel_cmdline/src/bytes.rs +++ b/crates/kernel_cmdline/src/bytes.rs @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; /// Wraps the raw command line bytes and provides methods for parsing and iterating /// over individual parameters. Uses copy-on-write semantics to avoid unnecessary /// allocations when working with borrowed data. -#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct Cmdline<'a>(Cow<'a, [u8]>); /// An owned Cmdline. Alias for `Cmdline<'static>`. @@ -375,6 +375,19 @@ impl<'a, 'other> Extend> for Cmdline<'a> { } } +impl PartialEq for Cmdline<'_> { + fn eq(&self, other: &Self) -> bool { + let mut our_params = self.iter().collect::>(); + our_params.sort(); + let mut their_params = other.iter().collect::>(); + their_params.sort(); + + our_params == their_params + } +} + +impl Eq for Cmdline<'_> {} + /// A single kernel command line parameter key /// /// Handles quoted values and treats dashes and underscores in keys as equivalent. @@ -1091,4 +1104,24 @@ mod tests { assert_eq!(params.len(), 0); } + + #[test] + fn test_cmdline_eq() { + // Ordering, quoting, and the whole dash-underscore + // equivalence thing shouldn't affect whether these are + // semantically equal + assert_eq!( + Cmdline::from("foo bar-with-delim=\"with spaces\""), + Cmdline::from("\"bar_with_delim=with spaces\" foo") + ); + + // Uneven lengths are not equal even if the parameters are. Or + // to put it another way, duplicate parameters break equality. + // Check with both orderings. + assert_ne!(Cmdline::from("foo"), Cmdline::from("foo foo")); + assert_ne!(Cmdline::from("foo foo"), Cmdline::from("foo")); + + // Equal lengths but differing duplicates are also not equal + assert_ne!(Cmdline::from("a a b"), Cmdline::from("a b b")); + } } diff --git a/crates/kernel_cmdline/src/utf8.rs b/crates/kernel_cmdline/src/utf8.rs index 49774a64a..408ea2c41 100644 --- a/crates/kernel_cmdline/src/utf8.rs +++ b/crates/kernel_cmdline/src/utf8.rs @@ -928,4 +928,24 @@ mod tests { assert_eq!(params[1], param("baz=qux")); assert_eq!(params[2], param("wiz")); } + + #[test] + fn test_cmdline_eq() { + // Ordering, quoting, and the whole dash-underscore + // equivalence thing shouldn't affect whether these are + // semantically equal + assert_eq!( + Cmdline::from("foo bar-with-delim=\"with spaces\""), + Cmdline::from("\"bar_with_delim=with spaces\" foo") + ); + + // Uneven lengths are not equal even if the parameters are. Or + // to put it another way, duplicate parameters break equality. + // Check with both orderings. + assert_ne!(Cmdline::from("foo"), Cmdline::from("foo foo")); + assert_ne!(Cmdline::from("foo foo"), Cmdline::from("foo")); + + // Equal lengths but differing duplicates are also not equal + assert_ne!(Cmdline::from("a a b"), Cmdline::from("a b b")); + } }