Repository navigation
stty: clear previous values when setting grouped flags - #15240
ozenyunus35 wants to merge 3 commits into
Conversation
Settings such as cs5-cs8, nl0/nl1, crN, tabN, bsN, vtN and ffN select one value of a multi-bit field. They were applied by setting their bits on top of the current value, so the field was never cleared: `cs7` on a terminal in `cs8` was ignored, `cs5`/`tab0` (value 0) were no-ops and `tab2` after `tab1` produced a mix of both. This also affected the combination settings that use them, such as `evenp`, `oddp` and `sane`. Clear the field before setting the selected value. Fixes uutils#11963
|
GNU testsuite comparison: |
Linux pseudo-terminals always keep cs8, so the integration test for the CSIZE group failed there. Check that group in a unit test that applies the settings to a termios value instead, and keep the pseudo-terminal test for the TABDLY group.
| fn apply_flag<T: TermiosFlag>(termios: &mut Termios, setting: &Flag<T>, val: bool) { | ||
| // A grouped flag selects one value of a multi-bit field (e.g. `cs7` in CSIZE), | ||
| // so the bits of the previously selected value have to be cleared first. | ||
| if let (Some(group), true) = (setting.group, val) { |
There was a problem hiding this comment.
could be if val && let Some(group) = setting.group, easier to read, no?
There was a problem hiding this comment.
Switched to the let chain in 220b7fe, thanks.
Merging this PR will degrade performance by 20.37%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | three_39_bit_primes |
490.4 ms | 672 ms | -27.02% |
| ❌ | Simulation | five_38_bit_primes |
1.7 s | 1.9 s | -13.11% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ozenyunus35:fix/stty-grouped-flags (220b7fe) with main (efca981)2
Footnotes
-
54 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(d43f2b8) during the generation of this report, so efca981 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
This branch has not been deployed
2 participants
Summary
Fixes #11963.
Fix
sttysettings that represent mutually exclusive values within the same bit field.Previously, applying a new grouped setting only ORed its bits into the existing terminal flags. The previous value was not cleared, so some settings were silently ignored or produced invalid combinations.
For example:
cs8tocs7could leavecs8active.cs5andtab0could not replace an existing value.tab1followed bytab2could produce an invalid combined value.Changes
Update
apply_settingto clear the relevant group's existing bits before applying the new value.The change covers character-size and terminal delay settings, including the grouped settings used by
evenpandoddp.Add two integration tests using a real pseudo-terminal to verify that grouped values replace one another correctly.
Testing
Regression tests:
Validation:
test_stty: 53 passed, 32 ignored.uu_sttyunit tests: 26 passed.cargo fmt --check: passed.-D warnings: passed.The tests were run on macOS. Cross-platform behavior will also be covered by CI.
One intermittent test failure occurred during parallel validation, but it could not be reproduced in 30 subsequent runs.
Compatibility
This change intentionally updates grouped flag handling to match the expected GNU
sttybehavior.No unrelated command behavior is changed.
AI Assistance
AI assistance was used during implementation and testing. The changes were reviewed and validated before submission.