From 8b6f4b0892a04c98b021cd416c6160027a365d87 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Thu, 9 Jul 2026 09:44:28 +0200 Subject: [PATCH] option_parser: Split `Tuple` into `Tuple` and `TupleList` We split the existing `Tuple` type into two types, one that represents a single `Tuple` and one that represents a list of tuples. This allows us to test tuple parsing and the parsing of tuple lists via distinct unit tests in follow-up commits. The renaming also brings consistency by adapting the naming scheme of the other list types defined in `option_parser`, e.g. `IntegerList` and `StringList`. Signed-off-by: Pascal Scholz On-behalf-of: SAP pascal.scholz@sap.com --- option_parser/src/lib.rs | 115 +++++++++++++++++++++------------------ vmm/src/config.rs | 18 +++--- 2 files changed, 72 insertions(+), 61 deletions(-) diff --git a/option_parser/src/lib.rs b/option_parser/src/lib.rs index dc97ec1d8..6ab3314a7 100644 --- a/option_parser/src/lib.rs +++ b/option_parser/src/lib.rs @@ -272,7 +272,7 @@ impl OptionParser { /// /// `T` can be any type that implements `FromStr` (e.g. `u32`, `String`), /// or one of this crate's types such as [`Toggle`], [`IntegerList`], - /// [`Tuple`], or [`StringList`]. + /// [`Tuple`], [`TupleList`] or [`StringList`]. pub fn convert(&self, option: &str) -> OptionParserResult> { match self.options.get(option).and_then(|v| v.value.as_ref()) { None => Ok(None), @@ -464,62 +464,76 @@ impl TupleValue for Vec { } } -/// A list of `key@value` pairs parsed from a bracket-enclosed string. -/// -/// The format is `[key1@value1,key2@value2,...]` where `@` separates each -/// pair's elements. `S` is the key type and `T` is the value type. -#[derive(PartialEq, Eq, Debug)] -pub struct Tuple(pub Vec<(S, T)>); - #[derive(Error, Debug)] pub enum TupleError { #[error("invalid value: {0}")] InvalidValue(String), - #[error("split outside brackets")] - SplitOutsideBrackets(#[source] OptionParserError), + #[error("Unbalanced brackets in one of the values")] + SplitInsideBrackets(#[source] OptionParserError), + #[error("Expected a single pair of enclosing brackets in input: {0}")] + UnbalancedOutsideBrackets(String), #[error("invalid integer list")] InvalidIntegerList(#[source] IntegerListParseError), #[error("invalid integer")] InvalidInteger(#[source] ParseIntError), } +/// A tuple consisting of a `key@value` pair parsed from a string. +#[derive(PartialEq, Eq, Debug)] +pub struct Tuple(pub S, pub T); + +/// A list of `key@value` pairs parsed from a bracket-enclosed string. +/// +/// The format is `[key1@value1,key2@value2,...]` where `@` separates each +/// pair's elements. `S` is the key type and `T` is the value type. +#[derive(PartialEq, Eq, Debug)] +pub struct TupleList(pub Vec>); + impl Parseable for Tuple { type Err = TupleError; + fn from_str(tuple: &str) -> result::Result { + let mut in_quotes = false; + let mut last_idx = 0; + let mut first_val = None; + for (idx, c) in tuple.as_bytes().iter().enumerate() { + match c { + b'"' => in_quotes = !in_quotes, + b'@' if !in_quotes => { + if last_idx != 0 { + return Err(TupleError::InvalidValue((*tuple).to_string())); + } + first_val = Some(&tuple[last_idx..idx]); + last_idx = idx + 1; + } + _ => {} + } + } + let item1 = ::from_str( + first_val.ok_or(TupleError::InvalidValue((*tuple).to_string()))?, + ) + .map_err(|_| TupleError::InvalidValue(first_val.unwrap().to_owned()))?; + let item2: T = TupleValue::parse_value(&tuple[last_idx..])?; + Ok(Tuple(item1, item2)) + } +} + +impl Parseable for TupleList { + type Err = TupleError; + fn from_str(s: &str) -> result::Result { - let mut list: Vec<(S, T)> = Vec::new(); + let mut list: Vec> = Vec::new(); let body = s .trim() .strip_prefix('[') .and_then(|s| s.strip_suffix(']')) - .ok_or_else(|| TupleError::InvalidValue(s.to_string()))?; - let tuples_list = split_commas(body).map_err(TupleError::SplitOutsideBrackets)?; - for tuple in tuples_list.iter() { - let mut in_quotes = false; - let mut last_idx = 0; - let mut first_val = None; - for (idx, c) in tuple.as_bytes().iter().enumerate() { - match c { - b'"' => in_quotes = !in_quotes, - b'@' if !in_quotes => { - if last_idx != 0 { - return Err(TupleError::InvalidValue((*tuple).to_string())); - } - first_val = Some(&tuple[last_idx..idx]); - last_idx = idx + 1; - } - _ => {} - } - } - let item1 = ::from_str( - first_val.ok_or(TupleError::InvalidValue((*tuple).to_string()))?, - ) - .map_err(|_| TupleError::InvalidValue(first_val.unwrap().to_owned()))?; - let item2 = TupleValue::parse_value(&tuple[last_idx..])?; - list.push((item1, item2)); + .ok_or_else(|| TupleError::UnbalancedOutsideBrackets(s.to_string()))?; + let tuples_raw = split_commas(body).map_err(TupleError::SplitInsideBrackets)?; + for tuple_raw in tuples_raw.iter() { + list.push(Tuple::from_str(tuple_raw)?); } - Ok(Tuple(list)) + Ok(TupleList(list)) } } @@ -636,10 +650,10 @@ mod unit_tests { parser.parse("topology=[\"@\"\"b\"@[1,2]]").unwrap(); assert_eq!( parser - .convert::>>("topology") + .convert::>>("topology") .unwrap() .unwrap(), - Tuple(vec![("@\"b".to_owned(), vec![1, 2])]) + TupleList(vec![Tuple("@\"b".to_owned(), vec![1, 2])]) ); parser.parse("cmdline=\"console=ttyS0,9600n8\"").unwrap(); @@ -836,30 +850,27 @@ mod unit_tests { #[test] fn test_tuple_single_pair() { - let t = Tuple::::from_str("[foo@42]").unwrap(); - assert_eq!(t, Tuple(vec![("foo".to_owned(), 42)])); + let t = Tuple::::from_str("foo@42").unwrap(); + assert_eq!(t, Tuple("foo".to_owned(), 42)); + let t = Tuple::>::from_str("foo@[42]").unwrap(); + assert_eq!(t, Tuple("foo".to_owned(), vec![42])); } #[test] - fn test_tuple_multiple_pairs() { - let t = Tuple::>::from_str("[a@[1,2],b@[3,4]]").unwrap(); + fn test_tuple_list_multiple_pairs() { + let t = TupleList::>::from_str("[a@[1,2],b@[3,4]]").unwrap(); assert_eq!( t, - Tuple(vec![ - ("a".to_owned(), vec![1, 2]), - ("b".to_owned(), vec![3, 4]), + TupleList(vec![ + Tuple("a".to_owned(), vec![1, 2]), + Tuple("b".to_owned(), vec![3, 4]), ]) ); } #[test] fn test_tuple_missing_at_separator() { - Tuple::::from_str("[foo42]").unwrap_err(); - } - - #[test] - fn test_tuple_missing_brackets() { - Tuple::::from_str("foo@42").unwrap_err(); + Tuple::::from_str("foo42").unwrap_err(); } #[test] diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 900fd3173..3a5723149 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -16,7 +16,7 @@ use block::ImageType; use clap::ArgMatches; use log::{debug, warn}; use option_parser::{ - ByteSized, IntegerList, OptionParser, OptionParserError, StringList, Toggle, Tuple, + ByteSized, IntegerList, OptionParser, OptionParserError, StringList, Toggle, Tuple, TupleList, }; use pci::NUM_DEVICE_IDS; use serde::{Deserialize, Serialize}; @@ -744,11 +744,11 @@ impl CpusConfig { .map_err(Error::ParseCpus)? .unwrap_or(DEFAULT_MAX_PHYS_BITS); let affinity = parser - .convert::>>("affinity") + .convert::>>("affinity") .map_err(Error::ParseCpus)? .map(|v| { v.0.iter() - .map(|(e1, e2)| CpuAffinity { + .map(|Tuple(e1, e2)| CpuAffinity { vcpu: *e1, host_cpus: e2.clone().into_boxed_slice(), }) @@ -1529,11 +1529,11 @@ impl DiskConfig { .unwrap_or_default(); let serial = parser.get("serial"); let queue_affinity = parser - .convert::>>("queue_affinity") + .convert::>>("queue_affinity") .map_err(Error::ParseDisk)? .map(|v| { v.0.iter() - .map(|(e1, e2)| VirtQueueAffinity { + .map(|Tuple(e1, e2)| VirtQueueAffinity { queue_index: *e1, host_cpus: e2.clone().into_boxed_slice(), }) @@ -2654,11 +2654,11 @@ impl NumaConfig { .map_err(Error::ParseNuma)? .map(|v| v.0.iter().map(|e| *e as u32).collect()); let distances = parser - .convert::>("distances") + .convert::>("distances") .map_err(Error::ParseNuma)? .map(|v| { v.0.iter() - .map(|(e1, e2)| NumaDistance { + .map(|Tuple(e1, e2)| NumaDistance { destination: *e1 as u32, distance: *e2 as u8, }) @@ -2850,11 +2850,11 @@ impl RestoreConfig { .map_err(Error::ParseRestore)? .unwrap_or_default(); let net_fds = parser - .convert::>>("net_fds") + .convert::>>("net_fds") .map_err(Error::ParseRestore)? .map(|v| { v.0.iter() - .map(|(id, fds)| RestoredNetConfig { + .map(|Tuple(id, fds)| RestoredNetConfig { id: id.clone(), num_fds: fds.len(), fds: Some(fds.iter().map(|e| *e as i32).collect()),