From 62a39763866f08bbdfec490e0e8a33e4d367eecc Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Mon, 12 May 2025 11:42:51 -0400 Subject: [PATCH 01/12] reworked arg processing. control character mappings are correctly grouped now, ie 'stty erase ^H' --- src/uu/stty/src/stty.rs | 45 +++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 5b5a9948cd5..c18f5aff277 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -8,14 +8,14 @@ mod flags; use clap::{Arg, ArgAction, ArgMatches, Command}; -use nix::libc::{O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ, c_ushort}; +use nix::libc::{c_ushort, O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ}; use nix::sys::termios::{ - ControlFlags, InputFlags, LocalFlags, OutputFlags, SpecialCharacterIndices, Termios, - cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, + cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, ControlFlags, InputFlags, LocalFlags, + OutputFlags, SpecialCharacterIndices, Termios, }; use nix::{ioctl_read_bad, ioctl_write_ptr_bad}; use std::fs::File; -use std::io::{self, Stdout, stdout}; +use std::io::{self, stdout, Stdout}; use std::ops::ControlFlow; use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::fs::OpenOptionsExt; @@ -212,8 +212,27 @@ fn stty(opts: &Options) -> UResult<()> { let mut termios = tcgetattr(opts.file.as_fd()).expect("Could not get terminal attributes"); if let Some(settings) = &opts.settings { - for setting in settings { - if let ControlFlow::Break(false) = apply_setting(&mut termios, setting) { + let mut settings_iter = settings.into_iter(); + while let Some(setting) = settings_iter.next() { + if is_control_char(setting) { + let new_cc = match settings_iter.next() { + Some(cc) => cc, + None => { + return Err(USimpleError::new( + 1, + format!("no mapping specified for '{setting}'"), + )); + } + }; + if let ControlFlow::Break(false) = apply_char_mapping(&mut termios, setting, new_cc) + { + return Err(USimpleError::new( + 1, + format!("invalid mapping '{setting}' => '{new_cc}'"), + )); + } + } + else if let ControlFlow::Break(false) = apply_setting(&mut termios, setting) { return Err(USimpleError::new( 1, format!("invalid argument '{setting}'"), @@ -283,6 +302,15 @@ fn print_terminal_size(termios: &Termios, opts: &Options) -> nix::Result<()> { Ok(()) } +fn is_control_char(option: &str) -> bool { + for cc in CONTROL_CHARS { + if option == cc.0 { + return true; + } + } + false +} + fn control_char_to_string(cc: nix::libc::cc_t) -> nix::Result { if cc == 0 { return Ok("".to_string()); @@ -471,6 +499,11 @@ fn apply_baud_rate_flag(termios: &mut Termios, input: &str) -> ControlFlow ControlFlow::Continue(()) } +fn apply_char_mapping(termios: &mut Termios, cc: &str, new_cc: &str) -> ControlFlow { + println!("{cc} {new_cc}"); + ControlFlow::Break(true) +} + pub fn uu_app() -> Command { Command::new(uucore::util_name()) .version(uucore::crate_version!()) From ae0c61dc4c63b9156dcbd07b019ddb0cc91a1689 Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Mon, 12 May 2025 13:58:04 -0400 Subject: [PATCH 02/12] stty: setting control chars to undefined (disabling them) is implemented --- src/uu/stty/src/stty.rs | 60 ++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index c18f5aff277..74c8d52ccd2 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -212,27 +212,24 @@ fn stty(opts: &Options) -> UResult<()> { let mut termios = tcgetattr(opts.file.as_fd()).expect("Could not get terminal attributes"); if let Some(settings) = &opts.settings { - let mut settings_iter = settings.into_iter(); + let mut settings_iter = settings.iter(); while let Some(setting) = settings_iter.next() { - if is_control_char(setting) { - let new_cc = match settings_iter.next() { - Some(cc) => cc, - None => { - return Err(USimpleError::new( - 1, - format!("no mapping specified for '{setting}'"), - )); - } + if let Some(char_index) = is_control_char(setting) { + let Some(new_cc) = settings_iter.next() else { + return Err(USimpleError::new( + 1, + format!("no mapping specified for '{setting}'"), + )); }; - if let ControlFlow::Break(false) = apply_char_mapping(&mut termios, setting, new_cc) + if let ControlFlow::Break(false) = + apply_char_mapping(&mut termios, setting, char_index, new_cc) { return Err(USimpleError::new( 1, format!("invalid mapping '{setting}' => '{new_cc}'"), )); } - } - else if let ControlFlow::Break(false) = apply_setting(&mut termios, setting) { + } else if let ControlFlow::Break(false) = apply_setting(&mut termios, setting) { return Err(USimpleError::new( 1, format!("invalid argument '{setting}'"), @@ -302,13 +299,13 @@ fn print_terminal_size(termios: &Termios, opts: &Options) -> nix::Result<()> { Ok(()) } -fn is_control_char(option: &str) -> bool { +fn is_control_char(option: &str) -> Option { for cc in CONTROL_CHARS { if option == cc.0 { - return true; + return Some(cc.1); } } - false + None } fn control_char_to_string(cc: nix::libc::cc_t) -> nix::Result { @@ -499,9 +496,34 @@ fn apply_baud_rate_flag(termios: &mut Termios, input: &str) -> ControlFlow ControlFlow::Continue(()) } -fn apply_char_mapping(termios: &mut Termios, cc: &str, new_cc: &str) -> ControlFlow { - println!("{cc} {new_cc}"); - ControlFlow::Break(true) +fn apply_char_mapping( + termios: &mut Termios, + cc: &str, + control_char_index: SpecialCharacterIndices, + new_cc: &str, +) -> ControlFlow { + if let Some(val) = string_to_control_char(new_cc) { + termios.control_chars[control_char_index as usize] = val; + return ControlFlow::Break(true); + } + ControlFlow::Break(false) +} + +// GNU stty defines some valid values for the control character mappings +// 1. Standard character, can be a a single char (ie 'C') or hat notation (ie '^C') +// 2. Integer +// a. hexadecimal, prefixed by '0x' +// b. octal, prefixed by '0' +// c. decimal, no prefix +// 3. Disabling the control character: '^-' or 'undef' +// +// This function returns the ascii value of the given char, or None if the input cannot be parsed +fn string_to_control_char(s: &str) -> Option { + // try to parse int, then char + if s == "undef" || s == "^-" { + return Some(0); + } + None } pub fn uu_app() -> Command { From eab54eced88e6901b0e16da1a5a98aa8dd057fc2 Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Mon, 12 May 2025 17:53:01 -0400 Subject: [PATCH 03/12] setting control chars --- src/uu/stty/src/stty.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 74c8d52ccd2..0769712b7c2 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -222,7 +222,7 @@ fn stty(opts: &Options) -> UResult<()> { )); }; if let ControlFlow::Break(false) = - apply_char_mapping(&mut termios, setting, char_index, new_cc) + apply_char_mapping(&mut termios, char_index, new_cc) { return Err(USimpleError::new( 1, @@ -498,7 +498,6 @@ fn apply_baud_rate_flag(termios: &mut Termios, input: &str) -> ControlFlow fn apply_char_mapping( termios: &mut Termios, - cc: &str, control_char_index: SpecialCharacterIndices, new_cc: &str, ) -> ControlFlow { @@ -523,7 +522,31 @@ fn string_to_control_char(s: &str) -> Option { if s == "undef" || s == "^-" { return Some(0); } - None + + // try to parse integer (hex, octal, or decimal) + if let Some(hex) = s.strip_prefix("0x") { + return u8::from_str_radix(hex, 16).ok(); + } else if let Some(octal) = s.strip_prefix("0") { + return u8::from_str_radix(octal, 8).ok(); + } else if let Ok(decimal) = s.parse::() { + return Some(decimal); + } + + // try to parse ^ or just + let mut chars = s.chars(); + match (chars.next(), chars.next()) { + (Some('^'), Some(c)) if c.is_ascii_alphabetic() => { + // subract by '@' to turn the char into the ascii value of '^' + if c == '?' { + println!("{}", (c.to_ascii_uppercase() as u8).wrapping_sub(b'@')); + } + Some((c.to_ascii_uppercase() as u8).wrapping_sub(b'@')) + } + (Some(c), None) => { + Some(c as u8) + } + _ => None, + } } pub fn uu_app() -> Command { From 39b28b2638bbf869ed488596351594c8bee1bb02 Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Mon, 12 May 2025 20:38:50 -0400 Subject: [PATCH 04/12] stty: can now set control chars. need to improve checks on valid mappings --- src/uu/stty/src/stty.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 0769712b7c2..9745abcdd15 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -8,14 +8,14 @@ mod flags; use clap::{Arg, ArgAction, ArgMatches, Command}; -use nix::libc::{c_ushort, O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ}; +use nix::libc::{O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ, c_ushort}; use nix::sys::termios::{ - cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, ControlFlags, InputFlags, LocalFlags, - OutputFlags, SpecialCharacterIndices, Termios, + ControlFlags, InputFlags, LocalFlags, OutputFlags, SpecialCharacterIndices, Termios, + cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, }; use nix::{ioctl_read_bad, ioctl_write_ptr_bad}; use std::fs::File; -use std::io::{self, stdout, Stdout}; +use std::io::{self, Stdout, stdout}; use std::ops::ControlFlow; use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::fs::OpenOptionsExt; @@ -35,6 +35,8 @@ use uucore::locale::get_message; use flags::BAUD_RATES; use flags::{CONTROL_CHARS, CONTROL_FLAGS, INPUT_FLAGS, LOCAL_FLAGS, OUTPUT_FLAGS}; +const ASCII_DEL: u8 = 127; + #[derive(Clone, Copy, Debug)] pub struct Flag { name: &'static str, @@ -218,7 +220,7 @@ fn stty(opts: &Options) -> UResult<()> { let Some(new_cc) = settings_iter.next() else { return Err(USimpleError::new( 1, - format!("no mapping specified for '{setting}'"), + format!("missing argument to '{setting}'"), )); }; if let ControlFlow::Break(false) = @@ -499,9 +501,9 @@ fn apply_baud_rate_flag(termios: &mut Termios, input: &str) -> ControlFlow fn apply_char_mapping( termios: &mut Termios, control_char_index: SpecialCharacterIndices, - new_cc: &str, + new_val: &str, ) -> ControlFlow { - if let Some(val) = string_to_control_char(new_cc) { + if let Some(val) = string_to_control_char(new_val) { termios.control_chars[control_char_index as usize] = val; return ControlFlow::Break(true); } @@ -516,9 +518,8 @@ fn apply_char_mapping( // c. decimal, no prefix // 3. Disabling the control character: '^-' or 'undef' // -// This function returns the ascii value of the given char, or None if the input cannot be parsed +// This function returns the ascii value of valid control chars, or None if the input is invalid fn string_to_control_char(s: &str) -> Option { - // try to parse int, then char if s == "undef" || s == "^-" { return Some(0); } @@ -535,16 +536,15 @@ fn string_to_control_char(s: &str) -> Option { // try to parse ^ or just let mut chars = s.chars(); match (chars.next(), chars.next()) { - (Some('^'), Some(c)) if c.is_ascii_alphabetic() => { - // subract by '@' to turn the char into the ascii value of '^' + (Some('^'), Some(c)) => { + // special case: ascii value of '^?' is greater than '?' if c == '?' { - println!("{}", (c.to_ascii_uppercase() as u8).wrapping_sub(b'@')); + return Some(ASCII_DEL); } + // subract by '@' to turn the char into the ascii value of '^' Some((c.to_ascii_uppercase() as u8).wrapping_sub(b'@')) } - (Some(c), None) => { - Some(c as u8) - } + (Some(c), _) => Some(c as u8), _ => None, } } From 3789d0edb4ec4a7dfaf9e72a92df01dcedde3503 Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Mon, 12 May 2025 22:42:10 -0400 Subject: [PATCH 05/12] stty: matches GNU in what control character mappings are allowed --- src/uu/stty/src/stty.rs | 70 ++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 9745abcdd15..3266281f957 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -103,6 +103,11 @@ enum Device { Stdout(Stdout), } +enum ControlCharMappingError { + IntOutOfRange, + MultipleChars, +} + impl AsFd for Device { fn as_fd(&self) -> BorrowedFd<'_> { match self { @@ -223,13 +228,22 @@ fn stty(opts: &Options) -> UResult<()> { format!("missing argument to '{setting}'"), )); }; - if let ControlFlow::Break(false) = - apply_char_mapping(&mut termios, char_index, new_cc) - { - return Err(USimpleError::new( - 1, - format!("invalid mapping '{setting}' => '{new_cc}'"), - )); + match apply_char_mapping(&mut termios, char_index, new_cc) { + Ok(_) => {} + Err(e) => match e { + ControlCharMappingError::IntOutOfRange => { + return Err(USimpleError::new( + 1, + format!("invalid integer argument: '{new_cc}': Numerical result out of range"), + )); + } + ControlCharMappingError::MultipleChars => { + return Err(USimpleError::new( + 1, + format!("invalid integer argument: '{new_cc}'"), + )); + } + }, } } else if let ControlFlow::Break(false) = apply_setting(&mut termios, setting) { return Err(USimpleError::new( @@ -502,12 +516,14 @@ fn apply_char_mapping( termios: &mut Termios, control_char_index: SpecialCharacterIndices, new_val: &str, -) -> ControlFlow { - if let Some(val) = string_to_control_char(new_val) { - termios.control_chars[control_char_index as usize] = val; - return ControlFlow::Break(true); +) -> Result<(), ControlCharMappingError> { + match string_to_control_char(new_val) { + Ok(val) => { + termios.control_chars[control_char_index as usize] = val; + Ok(()) + } + Err(e) => Err(e), } - ControlFlow::Break(false) } // GNU stty defines some valid values for the control character mappings @@ -519,33 +535,43 @@ fn apply_char_mapping( // 3. Disabling the control character: '^-' or 'undef' // // This function returns the ascii value of valid control chars, or None if the input is invalid -fn string_to_control_char(s: &str) -> Option { +fn string_to_control_char(s: &str) -> Result { if s == "undef" || s == "^-" { - return Some(0); + return Ok(0); } + // check if the number is greater than 255, return ControlCharMappingError::IntOutOfRange // try to parse integer (hex, octal, or decimal) + let mut ascii_num: Option = None; if let Some(hex) = s.strip_prefix("0x") { - return u8::from_str_radix(hex, 16).ok(); + ascii_num = u32::from_str_radix(hex, 16).ok(); } else if let Some(octal) = s.strip_prefix("0") { - return u8::from_str_radix(octal, 8).ok(); - } else if let Ok(decimal) = s.parse::() { - return Some(decimal); + ascii_num = u32::from_str_radix(octal, 8).ok(); + } else if let Ok(decimal) = s.parse::() { + ascii_num = Some(decimal); } + if let Some(val) = ascii_num { + if val > 255 { + return Err(ControlCharMappingError::IntOutOfRange); + } else { + return Ok(val as u8); + } + } // try to parse ^ or just let mut chars = s.chars(); match (chars.next(), chars.next()) { (Some('^'), Some(c)) => { // special case: ascii value of '^?' is greater than '?' if c == '?' { - return Some(ASCII_DEL); + return Ok(ASCII_DEL); } // subract by '@' to turn the char into the ascii value of '^' - Some((c.to_ascii_uppercase() as u8).wrapping_sub(b'@')) + Ok((c.to_ascii_uppercase() as u8).wrapping_sub(b'@')) } - (Some(c), _) => Some(c as u8), - _ => None, + (Some(c), None) => Ok(c as u8), + (Some(_), Some(_)) => Err(ControlCharMappingError::MultipleChars), + _ => unreachable!("No arguments provided: must have been caught earlier"), } } From 66198579a9cc37b208eef11e7e2277a42e37c779 Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Tue, 13 May 2025 10:22:22 -0400 Subject: [PATCH 06/12] stty: run rustfmt and remove extra comments --- src/uu/stty/src/stty.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 3266281f957..db08849905a 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -234,7 +234,9 @@ fn stty(opts: &Options) -> UResult<()> { ControlCharMappingError::IntOutOfRange => { return Err(USimpleError::new( 1, - format!("invalid integer argument: '{new_cc}': Numerical result out of range"), + format!( + "invalid integer argument: '{new_cc}': Numerical result out of range" + ), )); } ControlCharMappingError::MultipleChars => { @@ -540,7 +542,6 @@ fn string_to_control_char(s: &str) -> Result { return Ok(0); } - // check if the number is greater than 255, return ControlCharMappingError::IntOutOfRange // try to parse integer (hex, octal, or decimal) let mut ascii_num: Option = None; if let Some(hex) = s.strip_prefix("0x") { @@ -566,7 +567,7 @@ fn string_to_control_char(s: &str) -> Result { if c == '?' { return Ok(ASCII_DEL); } - // subract by '@' to turn the char into the ascii value of '^' + // subtract by '@' to turn the char into the ascii value of '^' Ok((c.to_ascii_uppercase() as u8).wrapping_sub(b'@')) } (Some(c), None) => Ok(c as u8), From da94aa925b405e03bc9798d30e20a324557bc7fc Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Thu, 15 May 2025 10:00:32 -0400 Subject: [PATCH 07/12] stty: setting control char code review fixes --- src/uu/stty/src/stty.rs | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index db08849905a..80b49105cc1 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -8,14 +8,14 @@ mod flags; use clap::{Arg, ArgAction, ArgMatches, Command}; -use nix::libc::{O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ, c_ushort}; +use nix::libc::{c_ushort, O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ}; use nix::sys::termios::{ - ControlFlags, InputFlags, LocalFlags, OutputFlags, SpecialCharacterIndices, Termios, - cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, + cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, ControlFlags, InputFlags, LocalFlags, + OutputFlags, SpecialCharacterIndices, Termios, }; use nix::{ioctl_read_bad, ioctl_write_ptr_bad}; use std::fs::File; -use std::io::{self, Stdout, stdout}; +use std::io::{self, stdout, Stdout}; use std::ops::ControlFlow; use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::fs::OpenOptionsExt; @@ -221,32 +221,24 @@ fn stty(opts: &Options) -> UResult<()> { if let Some(settings) = &opts.settings { let mut settings_iter = settings.iter(); while let Some(setting) = settings_iter.next() { - if let Some(char_index) = is_control_char(setting) { + if let Some(char_index) = cc_to_index(setting) { let Some(new_cc) = settings_iter.next() else { return Err(USimpleError::new( 1, format!("missing argument to '{setting}'"), )); }; - match apply_char_mapping(&mut termios, char_index, new_cc) { - Ok(_) => {} - Err(e) => match e { - ControlCharMappingError::IntOutOfRange => { - return Err(USimpleError::new( - 1, - format!( - "invalid integer argument: '{new_cc}': Numerical result out of range" - ), - )); - } + apply_char_mapping(&mut termios, char_index, new_cc).map_err(|e| { + let message = match e { + ControlCharMappingError::IntOutOfRange => format!( + "invalid integer argument: '{new_cc}': Numerical result out of range" + ), ControlCharMappingError::MultipleChars => { - return Err(USimpleError::new( - 1, - format!("invalid integer argument: '{new_cc}'"), - )); + format!("invalid integer argument: '{new_cc}'") } - }, - } + }; + USimpleError::new(1, message) + })?; } else if let ControlFlow::Break(false) = apply_setting(&mut termios, setting) { return Err(USimpleError::new( 1, @@ -317,7 +309,7 @@ fn print_terminal_size(termios: &Termios, opts: &Options) -> nix::Result<()> { Ok(()) } -fn is_control_char(option: &str) -> Option { +fn cc_to_index(option: &str) -> Option { for cc in CONTROL_CHARS { if option == cc.0 { return Some(cc.1); From 061a4e4444397092e2badf95c191a70e70ee6f4c Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Thu, 15 May 2025 10:11:43 -0400 Subject: [PATCH 08/12] stty: fix rustfmt errors --- src/uu/stty/src/stty.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 80b49105cc1..8ba56299a32 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -8,14 +8,14 @@ mod flags; use clap::{Arg, ArgAction, ArgMatches, Command}; -use nix::libc::{c_ushort, O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ}; +use nix::libc::{O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ, c_ushort}; use nix::sys::termios::{ - cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, ControlFlags, InputFlags, LocalFlags, - OutputFlags, SpecialCharacterIndices, Termios, + ControlFlags, InputFlags, LocalFlags, OutputFlags, SpecialCharacterIndices, Termios, + cfgetospeed, cfsetospeed, tcgetattr, tcsetattr, }; use nix::{ioctl_read_bad, ioctl_write_ptr_bad}; use std::fs::File; -use std::io::{self, stdout, Stdout}; +use std::io::{self, Stdout, stdout}; use std::ops::ControlFlow; use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::fs::OpenOptionsExt; From a40c389d5d623991c5c10fb1cddc875eca6c2f4a Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Fri, 16 May 2025 16:29:11 -0400 Subject: [PATCH 09/12] stty: more small edits after review --- src/uu/stty/src/stty.rs | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 8ba56299a32..5408c11d691 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -511,13 +511,9 @@ fn apply_char_mapping( control_char_index: SpecialCharacterIndices, new_val: &str, ) -> Result<(), ControlCharMappingError> { - match string_to_control_char(new_val) { - Ok(val) => { - termios.control_chars[control_char_index as usize] = val; - Ok(()) - } - Err(e) => Err(e), - } + string_to_control_char(new_val).map(|val| { + termios.control_chars[control_char_index as usize] = val; + }) } // GNU stty defines some valid values for the control character mappings @@ -528,21 +524,20 @@ fn apply_char_mapping( // c. decimal, no prefix // 3. Disabling the control character: '^-' or 'undef' // -// This function returns the ascii value of valid control chars, or None if the input is invalid +// This function returns the ascii value of valid control chars, or ControlCharMappingError if invalid fn string_to_control_char(s: &str) -> Result { if s == "undef" || s == "^-" { return Ok(0); } // try to parse integer (hex, octal, or decimal) - let mut ascii_num: Option = None; - if let Some(hex) = s.strip_prefix("0x") { - ascii_num = u32::from_str_radix(hex, 16).ok(); + let ascii_num = if let Some(hex) = s.strip_prefix("0x") { + u32::from_str_radix(hex, 16).ok() } else if let Some(octal) = s.strip_prefix("0") { - ascii_num = u32::from_str_radix(octal, 8).ok(); - } else if let Ok(decimal) = s.parse::() { - ascii_num = Some(decimal); - } + u32::from_str_radix(octal, 8).ok() + } else { + s.parse::().ok() + }; if let Some(val) = ascii_num { if val > 255 { From 7c1d7bc50c832bcc7e1384b5b38f7f9c3ce874c5 Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Sat, 24 May 2025 13:33:15 -0400 Subject: [PATCH 10/12] stty: refactor set control char changes for better testing --- src/uu/stty/src/flags.rs | 25 ++++ src/uu/stty/src/stty.rs | 250 +++++++++++++++++++++++-------------- tests/by-util/test_stty.rs | 33 +++++ 3 files changed, 216 insertions(+), 92 deletions(-) diff --git a/src/uu/stty/src/flags.rs b/src/uu/stty/src/flags.rs index 79c85ceb257..d08029b5f17 100644 --- a/src/uu/stty/src/flags.rs +++ b/src/uu/stty/src/flags.rs @@ -26,6 +26,31 @@ use nix::sys::termios::{ SpecialCharacterIndices as S, }; +pub enum AllFlags<'a> { + #[cfg(any( + target_os = "freebsd", + target_os = "dragonfly", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + ))] + Baud(u32), + #[cfg(not(any( + target_os = "freebsd", + target_os = "dragonfly", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + )))] + Baud(BaudRate), + ControlFlags((&'a Flag, bool)), + InputFlags((&'a Flag, bool)), + LocalFlags((&'a Flag, bool)), + OutputFlags((&'a Flag, bool)), +} + pub const CONTROL_FLAGS: &[Flag] = &[ Flag::new("parenb", C::PARENB), Flag::new("parodd", C::PARODD), diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 5408c11d691..3067c8c33b4 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -3,10 +3,11 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore clocal erange tcgetattr tcsetattr tcsanow tiocgwinsz tiocswinsz cfgetospeed cfsetospeed ushort vmin vtime +// spell-checker:ignore clocal erange tcgetattr tcsetattr tcsanow tiocgwinsz tiocswinsz cfgetospeed cfsetospeed ushort vmin vtime cflag lflag mod flags; +use crate::flags::AllFlags; use clap::{Arg, ArgAction, ArgMatches, Command}; use nix::libc::{O_NONBLOCK, TIOCGWINSZ, TIOCSWINSZ, c_ushort}; use nix::sys::termios::{ @@ -16,7 +17,6 @@ use nix::sys::termios::{ use nix::{ioctl_read_bad, ioctl_write_ptr_bad}; use std::fs::File; use std::io::{self, Stdout, stdout}; -use std::ops::ControlFlow; use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::{AsRawFd, RawFd}; @@ -108,6 +108,17 @@ enum ControlCharMappingError { MultipleChars, } +enum ArgOptions<'a> { + Flags(AllFlags<'a>), + Mapping((SpecialCharacterIndices, u8)), +} + +impl<'a> From> for ArgOptions<'a> { + fn from(flag: AllFlags<'a>) -> Self { + ArgOptions::Flags(flag) + } +} + impl AsFd for Device { fn as_fd(&self) -> BorrowedFd<'_> { match self { @@ -215,38 +226,68 @@ fn stty(opts: &Options) -> UResult<()> { )); } - // TODO: Figure out the right error message for when tcgetattr fails - let mut termios = tcgetattr(opts.file.as_fd()).expect("Could not get terminal attributes"); - - if let Some(settings) = &opts.settings { - let mut settings_iter = settings.iter(); - while let Some(setting) = settings_iter.next() { - if let Some(char_index) = cc_to_index(setting) { - let Some(new_cc) = settings_iter.next() else { - return Err(USimpleError::new( - 1, - format!("missing argument to '{setting}'"), - )); - }; - apply_char_mapping(&mut termios, char_index, new_cc).map_err(|e| { - let message = match e { - ControlCharMappingError::IntOutOfRange => format!( - "invalid integer argument: '{new_cc}': Numerical result out of range" - ), - ControlCharMappingError::MultipleChars => { - format!("invalid integer argument: '{new_cc}'") - } - }; - USimpleError::new(1, message) - })?; - } else if let ControlFlow::Break(false) = apply_setting(&mut termios, setting) { - return Err(USimpleError::new( - 1, - format!("invalid argument '{setting}'"), - )); + let mut valid_args: Vec = Vec::new(); + + if let Some(args) = &opts.settings { + let mut args_iter = args.iter(); + // iterate over args: skip to next arg if current one is a control char + while let Some(arg) = args_iter.next() { + // control char + if let Some(char_index) = cc_to_index(arg) { + if let Some(mapping) = args_iter.next() { + let cc_mapping = string_to_control_char(mapping).map_err(|e| { + let message = match e { + ControlCharMappingError::IntOutOfRange => format!( + "invalid integer argument: '{mapping}': Value too large for defined data type" + ), + ControlCharMappingError::MultipleChars => { + format!("invalid integer argument: '{mapping}'") + } + }; + USimpleError::new(1, message) + })?; + valid_args.push(ArgOptions::Mapping((char_index, cc_mapping))); + } else { + return Err(USimpleError::new(1, format!("missing argument to '{arg}'"))); + } + // non control char flag + } else if let Some(flag) = string_to_flag(arg) { + let mut remove_group = false; + match flag { + AllFlags::Baud(_) => {} + AllFlags::ControlFlags((flag, remove)) => { + remove_group = check_flag_group(flag, remove); + } + AllFlags::InputFlags((flag, remove)) => { + remove_group = check_flag_group(flag, remove); + } + AllFlags::LocalFlags((flag, remove)) => { + remove_group = check_flag_group(flag, remove); + } + AllFlags::OutputFlags((flag, remove)) => { + remove_group = check_flag_group(flag, remove); + } + } + if remove_group { + return Err(USimpleError::new(1, format!("invalid argument '{arg}'"))); + } + valid_args.push(flag.into()); + // not a valid control char or flag + } else { + return Err(USimpleError::new(1, format!("invalid argument '{arg}'"))); } } + // TODO: Figure out the right error message for when tcgetattr fails + let mut termios = tcgetattr(opts.file.as_fd()).expect("Could not get terminal attributes"); + + // iterate over valid_args, match on the arg type, do the matching apply function + for arg in &valid_args { + match arg { + ArgOptions::Mapping(mapping) => apply_char_mapping(&mut termios, mapping), + ArgOptions::Flags(flag) => apply_setting(&mut termios, flag), + } + } tcsetattr( opts.file.as_fd(), nix::sys::termios::SetArg::TCSANOW, @@ -254,11 +295,20 @@ fn stty(opts: &Options) -> UResult<()> { ) .expect("Could not write terminal attributes"); } else { + // TODO: Figure out the right error message for when tcgetattr fails + let termios = tcgetattr(opts.file.as_fd()).expect("Could not get terminal attributes"); print_settings(&termios, opts).expect("TODO: make proper error here from nix error"); } Ok(()) } +fn check_flag_group(flag: &Flag, remove: bool) -> bool { + if remove && flag.group.is_some() { + return true; + } + false +} + fn print_terminal_size(termios: &Termios, opts: &Options) -> nix::Result<()> { let speed = cfgetospeed(termios); @@ -318,6 +368,63 @@ fn cc_to_index(option: &str) -> Option { None } +// return Some(flag) if the input is a valid flag, None if not +fn string_to_flag(option: &str) -> Option { + // BSDs use a u32 for the baud rate, so any decimal number applies. + #[cfg(any( + target_os = "freebsd", + target_os = "dragonfly", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + ))] + if let Ok(n) = option.parse::() { + return Some(AllFlags::Baud(n)); + } + + #[cfg(not(any( + target_os = "freebsd", + target_os = "dragonfly", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + )))] + for (text, baud_rate) in BAUD_RATES { + if *text == option { + return Some(AllFlags::Baud(*baud_rate)); + } + } + + let (remove, name) = match option.strip_prefix('-') { + Some(s) => (true, s), + None => (false, option), + }; + + for cflag in CONTROL_FLAGS { + if name == cflag.name { + return Some(AllFlags::ControlFlags((cflag, remove))); + } + } + for iflag in INPUT_FLAGS { + if name == iflag.name { + return Some(AllFlags::InputFlags((iflag, remove))); + } + } + for lflag in LOCAL_FLAGS { + if name == lflag.name { + return Some(AllFlags::LocalFlags((lflag, remove))); + } + } + for oflag in OUTPUT_FLAGS { + if name == oflag.name { + return Some(AllFlags::OutputFlags((oflag, remove))); + } + } + None +} + fn control_char_to_string(cc: nix::libc::cc_t) -> nix::Result { if cc == 0 { return Ok("".to_string()); @@ -425,55 +532,25 @@ fn print_flags(termios: &Termios, opts: &Options, flags: &[Flag< } /// Apply a single setting -/// -/// The value inside the `Break` variant of the `ControlFlow` indicates whether -/// the setting has been applied. -fn apply_setting(termios: &mut Termios, s: &str) -> ControlFlow { - apply_baud_rate_flag(termios, s)?; - - let (remove, name) = match s.strip_prefix('-') { - Some(s) => (true, s), - None => (false, s), - }; - apply_flag(termios, CONTROL_FLAGS, name, remove)?; - apply_flag(termios, INPUT_FLAGS, name, remove)?; - apply_flag(termios, OUTPUT_FLAGS, name, remove)?; - apply_flag(termios, LOCAL_FLAGS, name, remove)?; - ControlFlow::Break(false) -} - -/// Apply a flag to a slice of flags -/// -/// The value inside the `Break` variant of the `ControlFlow` indicates whether -/// the setting has been applied. -fn apply_flag( - termios: &mut Termios, - flags: &[Flag], - input: &str, - remove: bool, -) -> ControlFlow { - for Flag { - name, flag, group, .. - } in flags - { - if input == *name { - // Flags with groups cannot be removed - // Since the name matches, we can short circuit and don't have to check the other flags. - if remove && group.is_some() { - return ControlFlow::Break(false); - } - // If there is a group, the bits for that group should be cleared before applying the flag - if let Some(group) = group { - group.apply(termios, false); - } - flag.apply(termios, !remove); - return ControlFlow::Break(true); +fn apply_setting(termios: &mut Termios, setting: &AllFlags) { + match setting { + AllFlags::Baud(_) => apply_baud_rate_flag(termios, setting), + AllFlags::ControlFlags((setting, disable)) => { + setting.flag.apply(termios, !disable); + } + AllFlags::InputFlags((setting, disable)) => { + setting.flag.apply(termios, !disable); + } + AllFlags::LocalFlags((setting, disable)) => { + setting.flag.apply(termios, !disable); + } + AllFlags::OutputFlags((setting, disable)) => { + setting.flag.apply(termios, !disable); } } - ControlFlow::Continue(()) } -fn apply_baud_rate_flag(termios: &mut Termios, input: &str) -> ControlFlow { +fn apply_baud_rate_flag(termios: &mut Termios, input: &AllFlags) { // BSDs use a u32 for the baud rate, so any decimal number applies. #[cfg(any( target_os = "freebsd", @@ -483,9 +560,8 @@ fn apply_baud_rate_flag(termios: &mut Termios, input: &str) -> ControlFlow target_os = "netbsd", target_os = "openbsd" ))] - if let Ok(n) = input.parse::() { + if let AllFlags::Baud(n) = input { cfsetospeed(termios, n).expect("Failed to set baud rate"); - return ControlFlow::Break(true); } // Other platforms use an enum. @@ -497,23 +573,13 @@ fn apply_baud_rate_flag(termios: &mut Termios, input: &str) -> ControlFlow target_os = "netbsd", target_os = "openbsd" )))] - for (text, baud_rate) in BAUD_RATES { - if *text == input { - cfsetospeed(termios, *baud_rate).expect("Failed to set baud rate"); - return ControlFlow::Break(true); - } + if let AllFlags::Baud(br) = input { + cfsetospeed(termios, *br).expect("Failed to set baud rate"); } - ControlFlow::Continue(()) } -fn apply_char_mapping( - termios: &mut Termios, - control_char_index: SpecialCharacterIndices, - new_val: &str, -) -> Result<(), ControlCharMappingError> { - string_to_control_char(new_val).map(|val| { - termios.control_chars[control_char_index as usize] = val; - }) +fn apply_char_mapping(termios: &mut Termios, mapping: &(SpecialCharacterIndices, u8)) { + termios.control_chars[mapping.0 as usize] = mapping.1; } // GNU stty defines some valid values for the control character mappings diff --git a/tests/by-util/test_stty.rs b/tests/by-util/test_stty.rs index 7ccc56e5dee..e9e455e3208 100644 --- a/tests/by-util/test_stty.rs +++ b/tests/by-util/test_stty.rs @@ -64,3 +64,36 @@ fn save_and_all() { "the options for verbose and stty-readable output styles are mutually exclusive", ); } + +#[test] +fn no_mapping() { + new_ucmd!() + .args(&["intr"]) + .fails() + .stderr_contains("missing argument to 'intr'"); +} + +#[test] +fn invalid_mapping() { + new_ucmd!() + .args(&["intr", "cc"]) + .fails() + .stderr_contains("invalid integer argument: 'cc'"); + + new_ucmd!() + .args(&["intr", "256"]) + .fails() + .stderr_contains("invalid integer argument: '256': Value too large for defined data type"); + + new_ucmd!() + .args(&["intr", "0x100"]) + .fails() + .stderr_contains( + "invalid integer argument: '0x100': Value too large for defined data type", + ); + + new_ucmd!() + .args(&["intr", "0400"]) + .fails() + .stderr_contains("invalid integer argument: '0400': Value too large for defined data type"); +} From c99dc3326aa6b9ff18b1ca1c1aee7191bf1104e4 Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Sat, 24 May 2025 14:00:06 -0400 Subject: [PATCH 11/12] stty: fix ci error --- src/uu/stty/src/stty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 3067c8c33b4..c800a525319 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -561,7 +561,7 @@ fn apply_baud_rate_flag(termios: &mut Termios, input: &AllFlags) { target_os = "openbsd" ))] if let AllFlags::Baud(n) = input { - cfsetospeed(termios, n).expect("Failed to set baud rate"); + cfsetospeed(termios, *n).expect("Failed to set baud rate"); } // Other platforms use an enum. From 7ce7b5cf9faa93c70e8bcc93279aa868acdc762a Mon Sep 17 00:00:00 2001 From: Will Shuttleworth Date: Tue, 3 Jun 2025 09:41:28 -0400 Subject: [PATCH 12/12] stty: fix issues from code review --- src/uu/stty/src/stty.rs | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index c800a525319..6fb06acb8c7 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -252,22 +252,13 @@ fn stty(opts: &Options) -> UResult<()> { } // non control char flag } else if let Some(flag) = string_to_flag(arg) { - let mut remove_group = false; - match flag { - AllFlags::Baud(_) => {} - AllFlags::ControlFlags((flag, remove)) => { - remove_group = check_flag_group(flag, remove); - } - AllFlags::InputFlags((flag, remove)) => { - remove_group = check_flag_group(flag, remove); - } - AllFlags::LocalFlags((flag, remove)) => { - remove_group = check_flag_group(flag, remove); - } - AllFlags::OutputFlags((flag, remove)) => { - remove_group = check_flag_group(flag, remove); - } - } + let remove_group = match flag { + AllFlags::Baud(_) => false, + AllFlags::ControlFlags((flag, remove)) => check_flag_group(flag, remove), + AllFlags::InputFlags((flag, remove)) => check_flag_group(flag, remove), + AllFlags::LocalFlags((flag, remove)) => check_flag_group(flag, remove), + AllFlags::OutputFlags((flag, remove)) => check_flag_group(flag, remove), + }; if remove_group { return Err(USimpleError::new(1, format!("invalid argument '{arg}'"))); } @@ -303,10 +294,7 @@ fn stty(opts: &Options) -> UResult<()> { } fn check_flag_group(flag: &Flag, remove: bool) -> bool { - if remove && flag.group.is_some() { - return true; - } - false + remove && flag.group.is_some() } fn print_terminal_size(termios: &Termios, opts: &Options) -> nix::Result<()> { @@ -397,10 +385,8 @@ fn string_to_flag(option: &str) -> Option { } } - let (remove, name) = match option.strip_prefix('-') { - Some(s) => (true, s), - None => (false, option), - }; + let remove = option.starts_with('-'); + let name = option.trim_start_matches('-'); for cflag in CONTROL_FLAGS { if name == cflag.name {