Skip to content

stty: verify tcsetattr applied all settings #10569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
littleKitchen wants to merge 4 commits into
base: main
Choose a base branch
from
Open

stty: verify tcsetattr applied all settings #10569

littleKitchen wants to merge 4 commits into from

Conversation

@littleKitchen
Copy link

@littleKitchen littleKitchen commented Jan 29, 2026

Summary

Fixes #10324

POSIX allows tcsetattr() to return success while only partially applying the requested settings:

The tcsetattr() function shall return successfully if it was able to perform any of the requested actions, even if some of the requested actions could not be performed.

This PR adds verification by reading back terminal settings after tcsetattr() and comparing them with the requested configuration.

Changes

  • Added termios_eq() function to compare two Termios structs (flags, control characters, and baud rates)
  • After tcsetattr(), call tcgetattr() and verify settings match
  • If settings don't match, exit with error: "<device>: unable to perform all requested operations"

Testing

All existing tests pass (cargo test -p uu_stty).

This matches GNU stty behavior, which performs tcgetattr() after setting and uses eq_mode() to detect partial application.

@ChrisDryden
Copy link
Collaborator

ChrisDryden commented Jan 29, 2026

Just for doing some local tests to validate, do you have some testing commands that you used to compare the gnu behavior?

Would it be possible to add an integration test for this as well?

@github-actions
Copy link

github-actions bot commented Jan 29, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/basenc/bounded-memory is now passing!

@littleKitchen
Copy link
Author

littleKitchen commented Jan 30, 2026

Thanks for the review! I've added an integration test in the latest commit.

Testing Commands (GNU vs uutils comparison)

The issue is that POSIX allows tcsetattr() to return success while only partially applying settings. To compare behavior:

# GNU stty - after tcsetattr, it reads back and compares with eq_mode()
# If settings don't match, exits with: "<device>: unable to perform all requested operations"

# Test setting multiple values at once:
stty -F /dev/pts/0 intr ^A quit ^B erase ^H
stty -F /dev/pts/0  # Verify all three are set

About the integration test

I added test_multiple_settings_verified which:

  1. Sets multiple control characters at once (intr, quit, erase)
  2. Verifies all settings were actually applied by reading them back
  3. Resets to sane defaults

This tests the normal success path of the verification logic.

Note: Triggering an actual partial tcsetattr failure is difficult in a test environment since modern systems typically apply all settings atomically. The verification is a safety net for edge cases (unusual terminals, resource constraints, etc.) that are hard to reproduce reliably in CI.

@github-actions
Copy link

github-actions bot commented Jan 30, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/basenc/bounded-memory is now passing!

@littleKitchen littleKitchen force-pushed the fix/stty-verify-tcsetattr branch from 26e9c96 to f5ba572 Compare January 30, 2026 02:57
@github-actions
Copy link

github-actions bot commented Jan 30, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/basenc/bounded-memory is now passing!

@sylvestre
Copy link
Contributor

sylvestre commented Jan 30, 2026

A bunch of tests are failing

@github-actions
Copy link

github-actions bot commented Jan 30, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/basenc/bounded-memory is now passing!

@littleKitchen littleKitchen force-pushed the fix/stty-verify-tcsetattr branch from 8e457a8 to b2388fb Compare January 31, 2026 00:34
@github-actions
Copy link

github-actions bot commented Jan 31, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

@littleKitchen
stty: verify tcsetattr applied all settings
906ea61 
POSIX allows tcsetattr() to return success while only partially
applying the requested settings. This change adds verification by
reading back the terminal settings after tcsetattr() and comparing
them with the requested configuration.

If the settings don't match, stty now exits with an error message:
"<device>: unable to perform all requested operations"

This matches GNU stty behavior, which uses tcgetattr() after setting
and compares with eq_mode() to detect partial application.

Changes:
- Added verify_termios_changes() function to compare termios structs
- Uses from_bits_truncate() to normalize flags for portable comparison
- Only verifies fields that were actually changed (requested != original)
- Added integration test for multiple settings verification

Fixes uutils#10324
@littleKitchen
fix: skip verification when restoring saved state
2603a6c 
Saved states may contain platform-specific flags that can't be perfectly
restored on all systems. Skip verification for saved state restoration
since it's inherently 'best effort' - users expect it to work across
platforms even if not every flag can be set.
@littleKitchen
chore: add cflags and lflags to cspell dictionary
eb25cee 
These are termios control flag field names (c_cflag and c_lflag)
used in the stty verification code.
@littleKitchen
chore: trigger CI
9f1af77
@littleKitchen littleKitchen force-pushed the fix/stty-verify-tcsetattr branch from 3841f58 to 9f1af77 Compare January 31, 2026 08:10
@github-actions
Copy link

github-actions bot commented Jan 31, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/follow-name (passes in this run but fails in the 'main' branch)

@littleKitchen
Copy link
Author

littleKitchen commented Jan 31, 2026

The failing test (test_cat_broken_pipe_nonzero_and_message) appears to be a flaky test on ARM - it's unrelated to this PR's stty changes. The test depends on signal/pipe timing which can be inconsistent on aarch64.

Could a maintainer please re-run the failed job? Thanks!

@littleKitchen
Copy link
Author

littleKitchen commented Jan 31, 2026

Still seeing flaky ARM failures, now a different test:

test_dd::test_block_lower - called Option::unwrap() on a None value at CmdResult::code

This is a dd test, completely unrelated to my stty changes. The process appears to have been killed by a signal (no exit code returned).

These ARM CI flakes seem systemic. Could a maintainer re-run or advise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

stty: no verification that tcsetattr applied all settings

3 participants

@littleKitchen @ChrisDryden @sylvestre