stty: Use from_bits_retain to preserve extended termios flag bits for compatibility with GNU #10181
Conversation
… compatibility with GNU
There was a problem hiding this comment.
Pull request overview
This PR updates the stty utility to preserve extended/high termios flag bits for improved compatibility with GNU stty. The change switches from from_bits_truncate to from_bits_retain when applying saved terminal states, ensuring that vendor-specific or newer kernel flags are not silently discarded.
Changes:
- Changed termios flag reconstruction to use
from_bits_retaininstead offrom_bits_truncate - Added comprehensive test coverage for extended flag bits preservation
- Updated comments to explain the rationale for using
from_bits_retain
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/uu/stty/src/stty.rs | Modified apply_saved_state function to use from_bits_retain for all four termios flag groups, preserving unknown bits that the nix crate may not define |
| tests/by-util/test_stty.rs | Added new test test_saved_state_preserves_extended_flags that validates acceptance of saved states with high flag bits set |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Want to start this with the caveat that the issue that is linked is caused by a separate issue and is fixed with the latest version of uutils. The question here is what happens when control bits are passed to the kernel that are not recognized, it turns out that the expectation of GNU's behaviour is to just pass it directly to the kernel and not do any truncation. When researching if there are any use cases for this, I could only find very niche use cases where this can occur and in this situations it wouldn't result in an error or a failure it would just drop that flag from being passed to the kernel. The test here is not testing whether the bits are being truncated because even if the bits were truncated it would not return the error message that was indicated. I think its still a good idea to swap the truncate to the retain for compatibility but drop that test. |
|
Could you please add a test to make sure we don't regress ? |
tests/by-util/test_flags.rs
Outdated
| @@ -0,0 +1,27 @@ | |||
| #[cfg(test)] | |||
There was a problem hiding this comment.
please don't create a new file.
do it tests/by-util/test_stty.rs
There was a problem hiding this comment.
apolgise for this,actually tbh its my first code with rust, I dont know that much of syntax and code style, thanks for the suggestions, I have moved that test in latest commit thanks
|
GNU testsuite comparison: |
tests/by-util/test_stty.rs
Outdated
| use nix::sys::termios::OutputFlags; | ||
|
|
||
| #[test] | ||
| fn test_flags_retain_unknown_bits() { |
There was a problem hiding this comment.
we should write the test high level - see how it is done elsewhere in the file
There was a problem hiding this comment.
we should write the test high level - see how it is done elsewhere in the file
okay sir, did just now
…se new command syntax
|
GNU testsuite comparison: |
1 similar comment
|
GNU testsuite comparison: |
3 participants
Fix stty termios acceptance.
This patch updates the
sttyimplementation to accept all valid POSIX termiosflag combinations that the GNU coreutils version tolerates.
stty -goutput on LinuxRelated Issue: #stty: termios restore fails on valid termios state input #10182