Skip to content

stty: support any baud on Linux, replace nix with rustix - #15299

Open
oech3 wants to merge 1 commit into
uutils:mainfrom
oech3:stty-unsafe
Open

oech3 wants to merge 1 commit into
uutils:mainfrom
oech3:stty-unsafe

Conversation

@oech3

@oech3 oech3 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

@codspeed

codspeed Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 24.56%

⚠️ 120 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

ℹ️ 1 benchmark has no flame graph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 398 untouched benchmarks
⏩ 70 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation factor_three_39_bit_primes 593.6 ms 1,494.2 ms -60.27%
❌ Simulation factor_thirteen_39_bit_primes 8.8 s 9.3 s -4.88%
⚡ Simulation factor_five_38_bit_primes 1.9 s 1.7 s +13.62%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing oech3:stty-unsafe (7e3e80e) with main (2e28cc3)2

Open in CodSpeed

Footnotes

  1. 70 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. ↩

  2. No successful run was found on main (f286c98) during the generation of this report, so 2e28cc3 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Comment thread src/uu/stty/Cargo.toml Outdated
Comment thread src/uu/stty/src/stty.rs Outdated
Comment thread src/uu/stty/src/stty.rs Outdated
Comment thread src/uu/stty/src/stty.rs Outdated
@oech3
oech3 marked this pull request as draft October 13, 2026 16:36
Comment thread src/uu/stty/src/stty.rs Outdated
@oech3
oech3 force-pushed the stty-unsafe branch 5 times, most recently from 8c0b529 to 39bc936 Compare October 13, 2026 17:09
@oech3 oech3 changed the title stty: remove unsafe & cfg stty: replace nix with rustix Oct 13, 2026
@oech3 oech3 changed the title stty: replace nix with rustix stty: support any baud on Linux, replace nix with rustix Oct 13, 2026
@oech3
oech3 force-pushed the stty-unsafe branch 2 times, most recently from ff11e70 to f100d7d Compare October 13, 2026 17:47
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

GNU testsuite comparison:

Note: The gnu test tests/stty/bad-speed is now being skipped but was previously passing.

@oech3
oech3 force-pushed the stty-unsafe branch 2 times, most recently from 9be3124 to d420658 Compare October 13, 2026 18:30
@oech3

This comment was marked as outdated.

@xtqqczze

Copy link
Copy Markdown
Collaborator

Would it be possible to separate the refactor replacing nix with rustix from the fix for #8719 into separate PRs?

@oech3

oech3 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

It is very difficult since rustix automatically supports any baud and nix and rustix cannot be mixed.

@oech3

This comment was marked as resolved.

@oech3

This comment was marked as outdated.

@xtqqczze

This comment was marked as resolved.

@oech3

This comment was marked as resolved.

Comment thread src/uu/stty/src/stty.rs Outdated
@oech3
oech3 force-pushed the stty-unsafe branch 2 times, most recently from b5e10de to a0b3f27 Compare October 16, 2026 00:15
Comment thread src/uu/stty/src/flags.rs Outdated
@oech3
oech3 force-pushed the stty-unsafe branch 5 times, most recently from a820e39 to 5745437 Compare October 16, 2026 10:39
Comment thread src/uu/stty/src/flags.rs Outdated

@xtqqczze xtqqczze left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens on Linux when arbitrary baud rates are not supported (i.e. with glibc < 2.42)? Do we panic, return an error, or fall back to a different behavior?

@oech3

oech3 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

This should work for all libcs since this does not use glibc's API and uses ioctl directly. But I don't have legacy glibc.
I'm not sure if stty 51 test works on the CI. Currently, most stty tests are skipped due to missing console.

@xtqqczze

Copy link
Copy Markdown
Collaborator

This should work for all libcs since this does not use glibc's API and uses ioctl directly.

We have downstream consumers using rustix/use-libc cfg, so stty uses the glibc API in that case?

@oech3

oech3 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

No. We still don't use same C-function with GNU which cause panic at nix.

@oech3

oech3 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

nix (thus libc) creates a struct using cfset*speed (missing support for any baud on legacy glibc) at user space RAM and passes it to ioctl.
rustix creates a struct by its own way and passes it to ioctl.

@oech3

oech3 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

On Ubuntu 24.04 runner (legacy glibc), I got

Run script -q -c "stty; echo '---'; stty 51; echo '---'; stty" /dev/null
speed 38400 baud;
line = 0;
-brkint
ixon
-imaxbel
---
---
speed 51 baud;
line = 0;
-brkint
ixon
-imaxbel

@xtqqczze

Copy link
Copy Markdown
Collaborator

I think I understand now: in Rustix tcgetattr, tcgetwinsize, tcsetattr, tcsetwinsize are implemented using syscalls rather than libc, even when the use_libc feature is enabled?

@oech3

oech3 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

use_libc uses C-ioctl wrapper. So we bypasses cfset*speed which caused panic. (Thus, stty 51 worked under scripts).

@oech3

oech3 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

So use_libc allows us LD_PRELOADing C-ioctl. But patching bad-speed.sh to intercept it changes meaning.

Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
@oech3
oech3 force-pushed the stty-unsafe branch 2 times, most recently from 692124b to 7e3e80e Compare October 17, 2026 03:02

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stty: support B460800 and B921600 baud rates stty: support arbitrary baudrate (at least on linux) as GNU coreutils 9.8

3 participants