kernel: Fix dash-equality to be on key, not parameter

We had use cases which were doing `iter().find(|v| v.key ==`
which would NOT do the `-_` insensitive comparison. Add a newtype
`ParameterKey` and move the comparison there.

This makes the API slightly more akward to use when inspecting
as one needs `.key.0` but eh.

Signed-off-by: Colin Walters <[email protected]>

Author: cgwalters

crates/lib/src/install.rs

@@ -1666,12 +1666,10 @@ fn find_root_args_to_inherit(cmdline: &Cmdline, root_info: &Filesystem) -> Resul
     let root = cmdline.find("root");
     // If we have a root= karg, then use that
     let (mount_spec, kargs) = if let Some(root) = root {
-        let rootflags = cmdline
-            .iter()
-            .find(|arg| arg.key == crate::kernel::ROOTFLAGS);
+        let rootflags = cmdline.find(crate::kernel::ROOTFLAGS);
         let inherit_kargs = cmdline
             .iter()
-            .filter(|arg| arg.key.starts_with(crate::kernel::INITRD_ARG_PREFIX));
+            .filter(|arg| arg.key.0.starts_with(crate::kernel::INITRD_ARG_PREFIX));
         (
             root.value_lossy(),
             rootflags

crates/lib/src/kernel.rs

@@ -59,29 +59,56 @@ impl<'a> Cmdline<'a> {
     /// Returns the first parameter matching the given key, or `None` if not found.
     /// Key comparison treats dashes and underscores as equivalent.
     pub fn find(&'a self, key: impl AsRef<[u8]>) -> Option<Parameter<'a>> {
-        let key = key.as_ref();
+        let key = ParameterKey(key.as_ref());
         self.iter().find(|p| p.key == key)
     }
 }
 
-/// A single kernel command line parameter.
+/// A single kernel command line parameter key
 ///
-/// Can represent either a simple switch (key only) or a key-value pair.
 /// Handles quoted values and treats dashes and underscores in keys as equivalent.
 #[derive(Debug, Eq)]
+pub(crate) struct ParameterKey<'a>(pub &'a [u8]);
+
+impl<'a> From<&'a [u8]> for ParameterKey<'a> {
+    fn from(value: &'a [u8]) -> Self {
+        Self(value)
+    }
+}
+
+/// A single kernel command line parameter.
+#[derive(Debug, PartialEq, Eq)]
 pub(crate) struct Parameter<'a> {
     /// The parameter key as raw bytes
-    pub key: &'a [u8],
+    pub key: ParameterKey<'a>,
     /// The parameter value as raw bytes, if present
     pub value: Option<&'a [u8]>,
 }
 
 impl<'a> Parameter<'a> {
+    /// Create a new parameter with the provided key and value.
+    #[cfg(test)]
+    pub fn new_kv<'k: 'a, 'v: 'a>(key: &'k [u8], value: &'v [u8]) -> Self {
+        Self {
+            key: ParameterKey(key),
+            value: Some(value),
+        }
+    }
+
+    /// Create a new parameter with the provided key.
+    #[cfg(test)]
+    pub fn new_key(key: &'a [u8]) -> Self {
+        Self {
+            key: ParameterKey(key),
+            value: None,
+        }
+    }
+
     /// Returns the key as a lossy UTF-8 string.
     ///
     /// Invalid UTF-8 sequences are replaced with the Unicode replacement character.
     pub fn key_lossy(&self) -> String {
-        String::from_utf8_lossy(self.key).to_string()
+        String::from_utf8_lossy(self.key.0).to_string()
     }
 
     /// Returns the value as a lossy UTF-8 string.
@@ -105,11 +132,12 @@ impl<'a, T: AsRef<[u8]> + ?Sized> From<&'a T> for Parameter<'a> {
 
         match equals {
             None => Self {
-                key: input,
+                key: ParameterKey(input),
                 value: None,
             },
             Some(i) => {
                 let (key, mut value) = input.split_at(i);
+                let key = ParameterKey(key);
 
                 // skip `=`, we know it's the first byte because we
                 // found it above
@@ -131,7 +159,7 @@ impl<'a, T: AsRef<[u8]> + ?Sized> From<&'a T> for Parameter<'a> {
     }
 }
 
-impl PartialEq for Parameter<'_> {
+impl PartialEq for ParameterKey<'_> {
     /// Compares two parameters for equality.
     ///
     /// Keys are compared with dashes and underscores treated as equivalent.
@@ -150,17 +178,9 @@ impl PartialEq for Parameter<'_> {
         //
         // For example, "foo" == "foobar" since the zipped iterator
         // only compares the first three chars.
-        let our_iter = self.key.iter().map(dedashed);
-        let other_iter = other.key.iter().map(dedashed);
-        if !our_iter.eq(other_iter) {
-            return false;
-        }
-
-        match (self.value, other.value) {
-            (Some(ours), Some(other)) => ours == other,
-            (None, None) => true,
-            _ => false,
-        }
+        let our_iter = self.0.iter().map(dedashed);
+        let other_iter = other.0.iter().map(dedashed);
+        our_iter.eq(other_iter)
     }
 }
 
@@ -194,11 +214,11 @@ mod tests {
     #[test]
     fn test_parameter_simple() {
         let switch = Parameter::from("foo");
-        assert_eq!(switch.key, b"foo");
+        assert_eq!(switch.key.0, b"foo");
         assert_eq!(switch.value, None);
 
         let kv = Parameter::from("bar=baz");
-        assert_eq!(kv.key, b"bar");
+        assert_eq!(kv.key.0, b"bar");
         assert_eq!(kv.value, Some(b"baz".as_slice()));
     }
 
@@ -214,7 +234,7 @@ mod tests {
 
         // quotes don't get removed from keys
         let p = Parameter::from("\"\"\"");
-        assert_eq!(p.key, b"\"\"\"");
+        assert_eq!(p.key.0, b"\"\"\"");
 
         // quotes only get stripped from the absolute ends of values
         let p = Parameter::from("foo=\"internal \" quotes \" are ok\"");
@@ -270,30 +290,14 @@ mod tests {
         let kargs = Cmdline::from(b"foo=bar,bar2 baz=fuz wiz".as_slice());
         let mut iter = kargs.iter();
 
-        assert_eq!(
-            iter.next(),
-            Some(Parameter {
-                key: b"foo",
-                value: Some(b"bar,bar2".as_slice())
-            })
-        );
-
-        assert_eq!(
-            iter.next(),
-            Some(Parameter {
-                key: b"baz",
-                value: Some(b"fuz".as_slice())
-            })
-        );
+        assert_eq!(iter.next(), Some(Parameter::new_kv(b"foo", b"bar,bar2")));
 
         assert_eq!(
             iter.next(),
-            Some(Parameter {
-                key: b"wiz",
-                value: None,
-            })
+            Some(Parameter::new_kv(b"baz", b"fuz".as_slice()))
         );
 
+        assert_eq!(iter.next(), Some(Parameter::new_key(b"wiz")));
         assert_eq!(iter.next(), None);
 
         // Test the find API
@@ -310,4 +314,25 @@ mod tests {
         // tests are running
         assert!(kargs.iter().count() > 0);
     }
+
+    #[test]
+    fn test_kargs_find_dash_hyphen() {
+        let kargs = Cmdline::from(b"a-b=1 a_b=2".as_slice());
+        // find should find the first one, which is a-b=1
+        let p = kargs.find("a_b").unwrap();
+        assert_eq!(p.key.0, b"a-b");
+        assert_eq!(p.value.unwrap(), b"1");
+        let p = kargs.find("a-b").unwrap();
+        assert_eq!(p.key.0, b"a-b");
+        assert_eq!(p.value.unwrap(), b"1");
+
+        let kargs = Cmdline::from(b"a_b=2 a-b=1".as_slice());
+        // find should find the first one, which is a_b=2
+        let p = kargs.find("a_b").unwrap();
+        assert_eq!(p.key.0, b"a_b");
+        assert_eq!(p.value.unwrap(), b"2");
+        let p = kargs.find("a-b").unwrap();
+        assert_eq!(p.key.0, b"a_b");
+        assert_eq!(p.value.unwrap(), b"2");
+    }
 }