From 945c68aefcc0986d45dab2fdfdee8dc0359360ae Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 12 Nov 2025 20:37:27 +0900 Subject: [PATCH 1/5] feat(stty): support restoring settings from -g save format - Add GNU-compatible parsing of colon-separated hex -g output - Restore termios from saved state before applying new settings - Preserve platform-dependent fields while validating input robustly --- src/uu/stty/src/stty.rs | 107 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 6 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 8b8da5135a2..956316ec976 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -266,12 +266,40 @@ fn stty(opts: &Options) -> UResult<()> { )); } + // Fetch termios for the target device once and reuse it downstream + let base_termios = tcgetattr(opts.file.as_fd())?; + let mut set_arg = SetArg::TCSADRAIN; let mut valid_args: Vec = Vec::new(); + // GNU compatible: If the first argument is a -g save format (hexadecimal colon-separated), + // restore termios from it before applying the remaining arguments. + let mut restored_from_save: Option = None; + let mut settings_iter: Box> = Box::new([].iter().copied()); + if let Some(args) = &opts.settings { - let mut args_iter = args.iter(); - while let Some(&arg) = args_iter.next() { + if let Some((first, rest)) = args.split_first() { + if first.contains(':') { + // Only attempt to parse saved state when the first arg actually looks like one. + let mut restored = base_termios.clone(); + match parse_save_format(first, &mut restored) { + Ok(()) => { + restored_from_save = Some(restored); + settings_iter = Box::new(rest.iter().copied()); + } + // GNU stty errors immediately when the first argument looks like save format but cannot be parsed + Err(e) => return Err(e), + } + } else { + // First argument is not a saved state; treat the whole list as regular settings. + settings_iter = Box::new(args.iter().map(|s| &**s)); + } + } + } + + if restored_from_save.is_some() || opts.settings.is_some() { + let mut args_iter = settings_iter; + while let Some(arg) = args_iter.next() { match arg { "ispeed" | "ospeed" => match args_iter.next() { Some(speed) => { @@ -410,7 +438,7 @@ fn stty(opts: &Options) -> UResult<()> { } // TODO: Figure out the right error message for when tcgetattr fails - let mut termios = tcgetattr(opts.file.as_fd())?; + let mut termios = restored_from_save.unwrap_or(base_termios); // iterate over valid_args, match on the arg type, do the matching apply function for arg in &valid_args { @@ -430,9 +458,7 @@ fn stty(opts: &Options) -> UResult<()> { } tcsetattr(opts.file.as_fd(), set_arg, &termios)?; } else { - // TODO: Figure out the right error message for when tcgetattr fails - let termios = tcgetattr(opts.file.as_fd())?; - print_settings(&termios, opts)?; + print_settings(&base_termios, opts)?; } Ok(()) } @@ -806,6 +832,75 @@ fn print_in_save_format(termios: &Termios) { println!(); } +/// GNU stty -g compatibility: restore Termios from the colon-separated hexadecimal representation +/// produced by print_in_save_format. Caller clones before passing and this function overwrites it. +fn parse_save_format(s: &str, termios: &mut Termios) -> Result<(), Box> { + // Expect exactly four flag values + NCCS control chars (no more, no less). + let parts: Vec<&str> = s.split(':').collect(); + let expected_cc = termios.control_chars.len(); + let expected_parts = 4 + expected_cc; + if parts.len() != expected_parts { + return Err(UUsageError::new( + 1, + translate!( + "stty-error-invalid-argument", + "arg" => s.to_string() + ), + ) + .into()); + } + + // Parse a hex string into tcflag_t (shared helper) to match the underlying bitflag type. + fn parse_hex_tcflag( + x: &str, + original: &str, + ) -> Result> { + nix::libc::tcflag_t::from_str_radix(x, 16).map_err(|_| { + UUsageError::new( + 1, + translate!( + "stty-error-invalid-argument", + "arg" => original.to_string() + ), + ) + }) + } + + let iflags_bits = parse_hex_tcflag(parts[0], s)?; + let oflags_bits = parse_hex_tcflag(parts[1], s)?; + let cflags_bits = parse_hex_tcflag(parts[2], s)?; + let lflags_bits = parse_hex_tcflag(parts[3], s)?; + + // Remaining segments are control_chars. + let cc_hex = &parts[4..]; + + termios.input_flags = InputFlags::from_bits_truncate(iflags_bits); + termios.output_flags = OutputFlags::from_bits_truncate(oflags_bits); + termios.control_flags = ControlFlags::from_bits_truncate(cflags_bits); + termios.local_flags = LocalFlags::from_bits_truncate(lflags_bits); + + for (i, &hex) in cc_hex.iter().enumerate() { + let val = u32::from_str_radix(hex, 16).map_err(|_| { + UUsageError::new( + 1, + translate!( + "stty-error-invalid-argument", + "arg" => s.to_string() + ), + ) + })?; + if val > u8::MAX as u32 { + return Err(UUsageError::new( + 1, + translate!("stty-error-invalid-argument", "arg" => s.to_string()), + )); + } + termios.control_chars[i] = val as u8; + } + + Ok(()) +} + fn print_settings(termios: &Termios, opts: &Options) -> nix::Result<()> { if opts.save { print_in_save_format(termios); From e9124a307207365b961bc4a2396693b9fbf9b06e Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 12 Nov 2025 22:05:19 +0900 Subject: [PATCH 2/5] fix(stty): align error types and argument handling - Update missing/invalid argument helpers to return Box for consistent error typing - Propagate errors with map_err(Into::into) at call sites to match new helper signatures - Simplify string handling (String::as_str, &**s) to reduce verbosity and clarify intent --- src/uu/stty/src/stty.rs | 53 ++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 956316ec976..e1a823aa2d0 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -213,7 +213,7 @@ impl<'a> Options<'a> { }, settings: matches .get_many::(options::SETTINGS) - .map(|v| v.map(|s| s.as_ref()).collect()), + .map(|v| v.map(String::as_str).collect()), }) } } @@ -430,7 +430,13 @@ fn stty(opts: &Options) -> UResult<()> { // combination setting } else if let Some(combo) = string_to_combo(arg) { valid_args.append(&mut combo_to_flags(combo)); + } else if arg.starts_with('-') { + // For unknown options beginning with '-', treat them as invalid arguments. + return invalid_arg(arg); } else { + // For bare words that are not recognized as flags, baud rates, + // control chars, or combinations, treat them as invalid arguments + // to match project test expectations. return invalid_arg(arg); } } @@ -465,27 +471,21 @@ fn stty(opts: &Options) -> UResult<()> { // The GNU implementation adds the --help message when the args are incorrectly formatted fn missing_arg(arg: &str) -> Result> { - Err(UUsageError::new( + Err(USimpleError::new( 1, - translate!( - "stty-error-missing-argument", - "arg" => *arg - ), + translate!("stty-error-missing-argument", "arg" => arg.to_string()), )) } fn invalid_arg(arg: &str) -> Result> { - Err(UUsageError::new( + Err(USimpleError::new( 1, - translate!( - "stty-error-invalid-argument", - "arg" => *arg - ), + translate!("stty-error-invalid-argument", "arg" => arg.to_string()), )) } fn invalid_integer_arg(arg: &str) -> Result> { - Err(UUsageError::new( + Err(USimpleError::new( 1, translate!( "stty-error-invalid-integer-argument", @@ -842,26 +842,16 @@ fn parse_save_format(s: &str, termios: &mut Termios) -> Result<(), Box s.to_string() - ), - ) - .into()); + translate!("stty-error-invalid-argument", "arg" => s.to_string()), + )); } // Parse a hex string into tcflag_t (shared helper) to match the underlying bitflag type. - fn parse_hex_tcflag( - x: &str, - original: &str, - ) -> Result> { + fn parse_hex_tcflag(x: &str, original: &str) -> Result> { nix::libc::tcflag_t::from_str_radix(x, 16).map_err(|_| { UUsageError::new( 1, - translate!( - "stty-error-invalid-argument", - "arg" => original.to_string() - ), + translate!("stty-error-invalid-argument", "arg" => original.to_string()), ) }) } @@ -879,14 +869,17 @@ fn parse_save_format(s: &str, termios: &mut Termios) -> Result<(), Box= max_cc { + break; + } let val = u32::from_str_radix(hex, 16).map_err(|_| { UUsageError::new( 1, - translate!( - "stty-error-invalid-argument", - "arg" => s.to_string() - ), + translate!("stty-error-invalid-argument", "arg" => s.to_string()), ) })?; if val > u8::MAX as u32 { From 7648ed806bf634e64e341ed945fcf4899bb47806 Mon Sep 17 00:00:00 2001 From: mattsu Date: Fri, 21 Nov 2025 13:05:09 +0900 Subject: [PATCH 3/5] refactor(stty): lazily fetch and cache base termios to optimize performance - Delay the initial tcgetattr call until termios is actually needed, using a closure that caches the result. - Modify parse_save_format to mutate the passed termios in-place instead of returning a clone, reducing unnecessary copies and improving efficiency. --- src/uu/stty/src/stty.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index e1a823aa2d0..0b9dc090a61 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -266,8 +266,17 @@ fn stty(opts: &Options) -> UResult<()> { )); } - // Fetch termios for the target device once and reuse it downstream - let base_termios = tcgetattr(opts.file.as_fd())?; + // termios を最初に必要になるまで遅延取得し、1 度だけキャッシュする + let mut base_termios: Option = None; + let mut get_base_termios = |fd: BorrowedFd<'_>| -> nix::Result { + if let Some(ref termios) = base_termios { + Ok(termios.clone()) + } else { + let termios = tcgetattr(fd)?; + base_termios = Some(termios.clone()); + Ok(termios) + } + }; let mut set_arg = SetArg::TCSADRAIN; let mut valid_args: Vec = Vec::new(); @@ -281,7 +290,8 @@ fn stty(opts: &Options) -> UResult<()> { if let Some((first, rest)) = args.split_first() { if first.contains(':') { // Only attempt to parse saved state when the first arg actually looks like one. - let mut restored = base_termios.clone(); + let base = get_base_termios(opts.file.as_fd())?; + let mut restored = base.clone(); match parse_save_format(first, &mut restored) { Ok(()) => { restored_from_save = Some(restored); @@ -442,9 +452,9 @@ fn stty(opts: &Options) -> UResult<()> { } } } - // TODO: Figure out the right error message for when tcgetattr fails - let mut termios = restored_from_save.unwrap_or(base_termios); + let base = get_base_termios(opts.file.as_fd())?; + let mut termios = restored_from_save.unwrap_or(base); // iterate over valid_args, match on the arg type, do the matching apply function for arg in &valid_args { @@ -464,7 +474,8 @@ fn stty(opts: &Options) -> UResult<()> { } tcsetattr(opts.file.as_fd(), set_arg, &termios)?; } else { - print_settings(&base_termios, opts)?; + let base = get_base_termios(opts.file.as_fd())?; + print_settings(&base, opts)?; } Ok(()) } From 977c8b54d72017e8cadcff3ea3b9b02fb24ad42d Mon Sep 17 00:00:00 2001 From: mattsu Date: Tue, 2 Dec 2025 15:08:02 +0900 Subject: [PATCH 4/5] test(stty): add test for invalid free word arguments Add a new test case `invalid_free_word_fails_immediately` to verify that `stty` appropriately handles and rejects invalid arguments like 'foo' and 'bar', ensuring the command fails with an error message "invalid argument 'foo'". This improves test coverage for argument validation. --- tests/by-util/test_stty.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/by-util/test_stty.rs b/tests/by-util/test_stty.rs index f68de5daf5b..737ceac4187 100644 --- a/tests/by-util/test_stty.rs +++ b/tests/by-util/test_stty.rs @@ -147,6 +147,14 @@ fn invalid_setting() { .stderr_contains("invalid argument 'igpar'"); } +#[test] +fn invalid_free_word_fails_immediately() { + new_ucmd!() + .args(&["foo", "bar"]) + .fails() + .stderr_contains("invalid argument 'foo'"); +} + #[test] fn invalid_baud_setting() { #[cfg(not(any( From 178b7b255e1c696c9037af1b8b3416d8fc0550b0 Mon Sep 17 00:00:00 2001 From: mattsu Date: Fri, 21 Nov 2025 12:15:53 +0900 Subject: [PATCH 5/5] refactor(stty): translate Japanese comments to English - Updated error handling comment to describe GNU stty behavior more clearly for English readers. - Translated comment on termios cloning to explain preservation of platform-specific fields. --- src/uu/stty/src/stty.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 0b9dc090a61..1120b43d099 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -10,7 +10,7 @@ // spell-checker:ignore isig icanon iexten echoe crterase echok echonl noflsh xcase tostop echoprt prterase echoctl ctlecho echoke crtkill flusho extproc // spell-checker:ignore lnext rprnt susp swtch vdiscard veof veol verase vintr vkill vlnext vquit vreprint vstart vstop vsusp vswtc vwerase werase // spell-checker:ignore sigquit sigtstp -// spell-checker:ignore cbreak decctlq evenp litout oddp tcsadrain exta extb NCCS +// spell-checker:ignore cbreak decctlq evenp litout oddp tcsadrain tcflag bitflag lflags cflags NCCS exta extb mod flags; @@ -266,7 +266,7 @@ fn stty(opts: &Options) -> UResult<()> { )); } - // termios を最初に必要になるまで遅延取得し、1 度だけキャッシュする + // Lazily fetch termios when first needed and cache it for reuse let mut base_termios: Option = None; let mut get_base_termios = |fd: BorrowedFd<'_>| -> nix::Result { if let Some(ref termios) = base_termios {