From 820b23c1590751c392868698a35ea8bc0cae116f Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sun, 3 Mar 2024 22:27:17 +0000 Subject: [PATCH] Improve CLI parsing logic --- src/cli/device.rs | 138 +++++++++++++++++++++++---------------------- src/cli/logfmt.rs | 10 ++-- src/cli/symlink.rs | 53 +++++++++-------- src/cli/timeout.rs | 8 +-- 4 files changed, 111 insertions(+), 98 deletions(-) diff --git a/src/cli/device.rs b/src/cli/device.rs index ffc0e0d..1eee415 100644 --- a/src/cli/device.rs +++ b/src/cli/device.rs @@ -6,11 +6,18 @@ use anyhow::{bail, ensure, Context, Error, Result}; use udev::Enumerator; #[derive(Clone)] -pub struct Device(usize, DeviceType); +pub struct Device { + parent_level: usize, + kind: DeviceKind, +} #[derive(Clone)] -pub enum DeviceType { - Usb(String, Option, Option), +pub enum DeviceKind { + Usb { + vid: String, + pid: Option, + serial: Option, + }, Syspath(PathBuf), Devnode(PathBuf), } @@ -19,65 +26,58 @@ fn is_hex4(val: &str) -> bool { val.len() == 4 && val.chars().all(|c| c.is_ascii_hexdigit()) } -fn is_alphanum(val: &str) -> bool { - val.len() >= 1 && val.chars().all(|c| c.is_ascii_alphanumeric()) -} - impl FromStr for Device { type Err = Error; - fn from_str(s: &str) -> Result { - let parts: Vec<_> = s.split(":").collect(); - - let parent_steps = parts - .iter() - .copied() - .take_while(|part| *part == "parent-of") - .count(); - let parts = &parts[parent_steps..]; - - ensure!( - parts.len() >= 2, - "Device format should be `[[parent-of:]*]:`, found `{s}`" - ); + fn from_str(mut s: &str) -> Result { + let mut parent_level = 0; + while let Some(remainder) = s.strip_prefix("parent-of:") { + s = remainder; + parent_level += 1; + } - let prefix = parts[0]; - let parts = &parts[1..]; - let dev = parts.join(":"); + let Some((kind, dev)) = s.split_once(':') else { + bail!("Device format should be `[[parent-of:]*]:`, found `{s}`"); + }; - let device = match prefix { + let device = match kind { "usb" => { - ensure!( - parts.len() >= 1 && parts.len() <= 3, - "Device format for usb should be `[:[:]]`, found `{dev}`." - ); - - let mut parts = parts.iter().copied(); + let mut parts = dev.split(':'); let vid = parts.next().unwrap(); let pid = parts.next(); let serial = parts.next(); + if parts.next().is_some() { + bail!( + "Device format for usb should be `[:[:]]`, found `{dev}`." + ); + } + ensure!( is_hex4(vid), "USB device VID should be a 4 digit hex number, found `{vid}`" ); - ensure!( - pid.is_none() || is_hex4(pid.unwrap()), - "USB device PID should be a 4 digit hex number, found `{}`", - pid.unwrap() - ); - ensure!( - serial.is_none() || is_alphanum(serial.unwrap()), - "USB device SERIAL should be alphanumeric, found `{}`", - serial.unwrap() - ); + if let Some(pid) = pid { + ensure!( + is_hex4(pid), + "USB device PID should be a 4 digit hex number, found `{}`", + pid + ); + } + if let Some(serial) = serial { + ensure!( + !serial.is_empty() && serial.chars().all(|c| c.is_ascii_alphanumeric()), + "USB device SERIAL should be alphanumeric, found `{}`", + serial + ); + } let vid = vid.to_ascii_lowercase(); let pid = pid.map(|s| s.to_ascii_lowercase()); let serial = serial.map(|s| s.to_owned()); - DeviceType::Usb(vid, pid, serial) + DeviceKind::Usb { vid, pid, serial } } "syspath" => { let path = PathBuf::from(&dev); @@ -86,42 +86,48 @@ impl FromStr for Device { "Syspath device PATH should be a directory path in /sys/**" ); - DeviceType::Syspath(path) + DeviceKind::Syspath(path) } "devnode" => { let path = PathBuf::from(&dev); ensure!( - path.is_absolute() && path.starts_with("/dev") && !dev.ends_with("/"), + path.is_absolute() && path.starts_with("/dev") && !dev.ends_with('/'), "Devnode device PATH should be a file path in /dev/**" ); - DeviceType::Devnode(path) + DeviceKind::Devnode(path) } _ => { - bail!("Device PREFIX should be one of `usb`, `syspath` or `devnode`, found `{prefix}`"); + bail!( + "Device PREFIX should be one of `usb`, `syspath` or `devnode`, found `{kind}`" + ); } }; - Ok(Device(parent_steps, device)) + Ok(Device { + parent_level, + kind: device, + }) } } -impl Display for DeviceType { +impl Display for DeviceKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match &self { - DeviceType::Usb(vid, Some(pid), Some(serial)) => { - write!(f, "usb:{vid}:{pid}:{serial}") - } - DeviceType::Usb(vid, Some(pid), None) => { - write!(f, "usb:{vid}:{pid}") - } - DeviceType::Usb(vid, None, _) => { - write!(f, "usb:{vid}") + DeviceKind::Usb { vid, pid, serial } => { + write!(f, "usb:{vid}")?; + if let Some(pid) = pid { + write!(f, ":{pid}")?; + } + if let Some(serial) = serial { + write!(f, ":{serial}")?; + } + Ok(()) } - DeviceType::Syspath(path) => { + DeviceKind::Syspath(path) => { write!(f, "syspath:{}", path.display()) } - DeviceType::Devnode(path) => { + DeviceKind::Devnode(path) => { write!(f, "devnode:{}", path.display()) } } @@ -130,17 +136,17 @@ impl Display for DeviceType { impl Display for Device { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for _ in 0..self.0 { + for _ in 0..self.parent_level { write!(f, "parent-of:")?; } - write!(f, "{}", self.1) + write!(f, "{}", self.kind) } } -impl DeviceType { +impl DeviceKind { fn device(&self) -> Result { let dev = match &self { - DeviceType::Usb(vid, pid, serial) => { + DeviceKind::Usb { vid, pid, serial } => { let mut enumerator = Enumerator::new()?; enumerator.match_attribute("idVendor", vid)?; if let Some(pid) = pid { @@ -154,14 +160,14 @@ impl DeviceType { .next() .with_context(|| format!("Failed to find device `{self}`"))? } - DeviceType::Syspath(path) => { + DeviceKind::Syspath(path) => { let path = path .canonicalize() .with_context(|| format!("Failed to resolve PATH for `{self}`"))?; udev::Device::from_syspath(&path) .with_context(|| format!("Failed to find device `{self}`"))? } - DeviceType::Devnode(path) => { + DeviceKind::Devnode(path) => { let path = path .canonicalize() .with_context(|| format!("Failed to resolve PATH for `{self}`"))?; @@ -179,13 +185,13 @@ impl DeviceType { impl Device { pub fn device(&self) -> Result { - let device = self.1.device()?; + let device = self.kind.device()?; Ok(device) } pub fn hub(&self) -> Result { let mut device = self.device()?; - for _ in 0..self.0 { + for _ in 0..self.parent_level { device = device.parent().with_context(|| { format!("Failed to obtain parent device while resolving `{self}`") })?; diff --git a/src/cli/logfmt.rs b/src/cli/logfmt.rs index 5e72b4f..0197448 100644 --- a/src/cli/logfmt.rs +++ b/src/cli/logfmt.rs @@ -1,6 +1,7 @@ -use std::{fmt::Display, str::FromStr}; +use std::fmt::Display; +use std::str::FromStr; -use anyhow::bail; +use anyhow::{bail, Error, Result}; #[derive(Clone)] pub struct LogFormat { @@ -22,8 +23,9 @@ impl Default for LogFormat { } impl FromStr for LogFormat { - type Err = anyhow::Error; - fn from_str(s: &str) -> Result { + type Err = Error; + + fn from_str(s: &str) -> Result { let mut result = Self::default(); let mut value = true; for c in s.chars() { diff --git a/src/cli/symlink.rs b/src/cli/symlink.rs index b826902..fb08e5e 100644 --- a/src/cli/symlink.rs +++ b/src/cli/symlink.rs @@ -5,25 +5,27 @@ use anyhow::{bail, ensure, Error, Result}; #[derive(Clone)] pub enum SymlinkDevice { - Usb(String, String, String), + Usb { + vid: String, + pid: String, + if_num: String, + }, } #[derive(Clone)] -pub struct Symlink(SymlinkDevice, PathBuf); +pub struct Symlink { + device: SymlinkDevice, + path: PathBuf, +} fn is_hex4(val: &str) -> bool { val.len() == 4 && val.chars().all(|c| c.is_ascii_hexdigit()) } -fn is_number(val: &str) -> bool { - val.len() >= 1 && val.chars().all(|c| c.is_ascii_digit()) -} - impl FromStr for Symlink { type Err = Error; fn from_str(s: &str) -> Result { - let parts: Vec<_> = s.split("=").collect(); - + let parts: Vec<_> = s.split('=').collect(); ensure!( parts.len() == 2, "Symlink format should be `:=`, found `{s}`" @@ -33,24 +35,24 @@ impl FromStr for Symlink { let path = parts[1]; ensure!( - path.starts_with("/") && !path.ends_with("/"), + path.starts_with('/') && !path.ends_with('/'), "Symlink PATH should be an absolute file path, found `{path}`." ); let path = PathBuf::from(path); - let mut parts = dev.split(":"); - let prefix = parts.next().unwrap(); - let parts: Vec<_> = parts.collect(); - let dev = parts.join(":"); + let Some((kind, dev)) = s.split_once(':') else { + bail!("Symlink DEVICE format should be `:`, found `{dev}`"); + }; - match prefix { + match kind { "usb" => { + let parts: Vec<_> = dev.split(':').collect(); ensure!(parts.len() == 3, "Symlink DEVICE format for usb should be `::`, found `{dev}`."); let vid = parts[0]; let pid = parts[1]; - let ifc = parts[2]; + let if_num = parts[2]; ensure!( is_hex4(vid), @@ -61,18 +63,21 @@ impl FromStr for Symlink { "USB symlink PID should be a 4 digit hex number, found `{pid}`" ); ensure!( - is_number(ifc), - "USB symlink INTERFACE should be a number, found `{ifc}`" + !if_num.is_empty() && if_num.chars().all(|c| c.is_ascii_digit()), + "USB symlink INTERFACE should be a number, found `{if_num}`" ); let vid = vid.to_ascii_lowercase(); let pid = pid.to_ascii_lowercase(); - let ifc = format!("{ifc:0>2}"); + let if_num = format!("{if_num:0>2}"); - Ok(Symlink(SymlinkDevice::Usb(vid, pid, ifc), path)) + Ok(Symlink { + device: SymlinkDevice::Usb { vid, pid, if_num }, + path, + }) } _ => { - bail!("Symlink PREFIX should be `usb`, found `{prefix}`"); + bail!("Symlink PREFIX should be `usb`, found `{kind}`"); } } } @@ -81,10 +86,10 @@ impl FromStr for Symlink { impl SymlinkDevice { fn matches_impl(&self, device: &udev::Device) -> Option { let matches = match self { - SymlinkDevice::Usb(vid, pid, ifc) => { + SymlinkDevice::Usb { vid, pid, if_num } => { device.property_value("ID_VENDOR_ID")?.to_str()? == vid && device.property_value("ID_MODEL_ID")?.to_str()? == pid - && device.property_value("ID_USB_INTERFACE_NUM")?.to_str()? == ifc + && device.property_value("ID_USB_INTERFACE_NUM")?.to_str()? == if_num } }; Some(matches) @@ -97,8 +102,8 @@ impl SymlinkDevice { impl Symlink { pub fn matches(&self, device: &udev::Device) -> Option { - if self.0.matches(device) { - Some(self.1.clone()) + if self.device.matches(device) { + Some(self.path.clone()) } else { None } diff --git a/src/cli/timeout.rs b/src/cli/timeout.rs index 380f041..08c6f2c 100644 --- a/src/cli/timeout.rs +++ b/src/cli/timeout.rs @@ -9,9 +9,9 @@ pub enum Timeout { impl FromStr for Timeout { type Err = humantime::DurationError; fn from_str(s: &str) -> Result { - if s == "inf" || s == "infinite" || s == "none" || s == "forever" { - return Ok(Timeout::Infinite); - } - Ok(Timeout::Some(humantime::parse_duration(s)?)) + Ok(match s { + "inf" | "infinite" | "none" | "forever" => Timeout::Infinite, + _ => Timeout::Some(humantime::parse_duration(s)?), + }) } }