Skip to content

Commit 91d8861

Browse files
ChrisDrydensylvestre
authored andcommitted
stty: use stdin for TTY operations instead of /dev/tty (uutils#9881)
* stty: use stdin for TTY operations instead of /dev/tty * Add tests for stty stdin behavior and fix platform-specific error messages --------- Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
1 parent 89f7718 commit 91d8861

File tree

3 files changed

+105
-37
lines changed

3 files changed

+105
-37
lines changed

.vscode/cspell.dictionaries/workspace.wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ istrip
379379
litout
380380
opost
381381
parodd
382+
ENOTTY
382383

383384
# translation tests
384385
CLICOLOR

src/uu/stty/src/stty.rs

Lines changed: 34 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ use nix::sys::termios::{
2626
use nix::{ioctl_read_bad, ioctl_write_ptr_bad};
2727
use std::cmp::Ordering;
2828
use std::fs::File;
29-
use std::io::{self, Stdout, stdout};
29+
use std::io::{self, Stdin, stdin, stdout};
3030
use std::num::IntErrorKind;
3131
use std::os::fd::{AsFd, BorrowedFd};
3232
use std::os::unix::fs::OpenOptionsExt;
3333
use std::os::unix::io::{AsRawFd, RawFd};
34-
use uucore::error::{UError, UResult, USimpleError, UUsageError};
34+
use uucore::error::{FromIo, UError, UResult, USimpleError, UUsageError};
3535
use uucore::format_usage;
3636
use uucore::parser::num_parser::ExtendedParser;
3737
use uucore::translate;
@@ -124,12 +124,13 @@ struct Options<'a> {
124124
all: bool,
125125
save: bool,
126126
file: Device,
127+
device_name: String,
127128
settings: Option<Vec<&'a str>>,
128129
}
129130

130131
enum Device {
131132
File(File),
132-
Stdout(Stdout),
133+
Stdin(Stdin),
133134
}
134135

135136
#[derive(Debug)]
@@ -166,7 +167,7 @@ impl AsFd for Device {
166167
fn as_fd(&self) -> BorrowedFd<'_> {
167168
match self {
168169
Self::File(f) => f.as_fd(),
169-
Self::Stdout(stdout) => stdout.as_fd(),
170+
Self::Stdin(stdin) => stdin.as_fd(),
170171
}
171172
}
172173
}
@@ -175,45 +176,42 @@ impl AsRawFd for Device {
175176
fn as_raw_fd(&self) -> RawFd {
176177
match self {
177178
Self::File(f) => f.as_raw_fd(),
178-
Self::Stdout(stdout) => stdout.as_raw_fd(),
179+
Self::Stdin(stdin) => stdin.as_raw_fd(),
179180
}
180181
}
181182
}
182183

183184
impl<'a> Options<'a> {
184185
fn from(matches: &'a ArgMatches) -> io::Result<Self> {
185-
Ok(Self {
186-
all: matches.get_flag(options::ALL),
187-
save: matches.get_flag(options::SAVE),
188-
file: match matches.get_one::<String>(options::FILE) {
189-
// Two notes here:
190-
// 1. O_NONBLOCK is needed because according to GNU docs, a
191-
// POSIX tty can block waiting for carrier-detect if the
192-
// "clocal" flag is not set. If your TTY is not connected
193-
// to a modem, it is probably not relevant though.
194-
// 2. We never close the FD that we open here, but the OS
195-
// will clean up the FD for us on exit, so it doesn't
196-
// matter. The alternative would be to have an enum of
197-
// BorrowedFd/OwnedFd to handle both cases.
198-
Some(f) => Device::File(
186+
let (file, device_name) = match matches.get_one::<String>(options::FILE) {
187+
// Two notes here:
188+
// 1. O_NONBLOCK is needed because according to GNU docs, a
189+
// POSIX tty can block waiting for carrier-detect if the
190+
// "clocal" flag is not set. If your TTY is not connected
191+
// to a modem, it is probably not relevant though.
192+
// 2. We never close the FD that we open here, but the OS
193+
// will clean up the FD for us on exit, so it doesn't
194+
// matter. The alternative would be to have an enum of
195+
// BorrowedFd/OwnedFd to handle both cases.
196+
Some(f) => (
197+
Device::File(
199198
std::fs::OpenOptions::new()
200199
.read(true)
201200
.custom_flags(O_NONBLOCK)
202201
.open(f)?,
203202
),
204-
// default to /dev/tty, if that does not exist then default to stdout
205-
None => {
206-
if let Ok(f) = std::fs::OpenOptions::new()
207-
.read(true)
208-
.custom_flags(O_NONBLOCK)
209-
.open("/dev/tty")
210-
{
211-
Device::File(f)
212-
} else {
213-
Device::Stdout(stdout())
214-
}
215-
}
216-
},
203+
f.clone(),
204+
),
205+
// Per POSIX, stdin is used for TTY operations when no device is specified.
206+
// This matches GNU coreutils behavior: if stdin is not a TTY,
207+
// tcgetattr will fail with "Inappropriate ioctl for device".
208+
None => (Device::Stdin(stdin()), "standard input".to_string()),
209+
};
210+
Ok(Self {
211+
all: matches.get_flag(options::ALL),
212+
save: matches.get_flag(options::SAVE),
213+
file,
214+
device_name,
217215
settings: matches
218216
.get_many::<String>(options::SETTINGS)
219217
.map(|v| v.map(|s| s.as_ref()).collect()),
@@ -412,8 +410,8 @@ fn stty(opts: &Options) -> UResult<()> {
412410
}
413411
}
414412

415-
// TODO: Figure out the right error message for when tcgetattr fails
416-
let mut termios = tcgetattr(opts.file.as_fd())?;
413+
let mut termios =
414+
tcgetattr(opts.file.as_fd()).map_err_context(|| opts.device_name.clone())?;
417415

418416
// iterate over valid_args, match on the arg type, do the matching apply function
419417
for arg in &valid_args {
@@ -433,8 +431,7 @@ fn stty(opts: &Options) -> UResult<()> {
433431
}
434432
tcsetattr(opts.file.as_fd(), set_arg, &termios)?;
435433
} else {
436-
// TODO: Figure out the right error message for when tcgetattr fails
437-
let termios = tcgetattr(opts.file.as_fd())?;
434+
let termios = tcgetattr(opts.file.as_fd()).map_err_context(|| opts.device_name.clone())?;
438435
print_settings(&termios, opts)?;
439436
}
440437
Ok(())
@@ -997,7 +994,7 @@ fn apply_char_mapping(termios: &mut Termios, mapping: &(S, u8)) {
997994
///
998995
/// The state array contains:
999996
/// - `state[0]`: input flags
1000-
/// - `state[1]`: output flags
997+
/// - `state[1]`: output flags
1001998
/// - `state[2]`: control flags
1002999
/// - `state[3]`: local flags
10031000
/// - `state[4..]`: control characters (optional)

tests/by-util/test_stty.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,6 +1557,76 @@ fn test_saved_state_with_control_chars() {
15571557
.code_is(exp_result.code());
15581558
}
15591559

1560+
// Per POSIX, stty uses stdin for TTY operations. When stdin is a pipe, it should fail.
1561+
#[test]
1562+
#[cfg(unix)]
1563+
fn test_stdin_not_tty_fails() {
1564+
// ENOTTY error message varies by platform/libc:
1565+
// - glibc: "Inappropriate ioctl for device"
1566+
// - musl: "Not a tty"
1567+
// - Android: "Not a typewriter"
1568+
#[cfg(target_os = "android")]
1569+
let expected_error = "standard input: Not a typewriter";
1570+
#[cfg(all(not(target_os = "android"), target_env = "musl"))]
1571+
let expected_error = "standard input: Not a tty";
1572+
#[cfg(all(not(target_os = "android"), not(target_env = "musl")))]
1573+
let expected_error = "standard input: Inappropriate ioctl for device";
1574+
1575+
new_ucmd!()
1576+
.pipe_in("")
1577+
.fails()
1578+
.stderr_contains(expected_error);
1579+
}
1580+
1581+
// Test that stty uses stdin for TTY operations per POSIX.
1582+
// Verifies: output redirection (#8012), save/restore pattern (#8608), stdin redirection (#8848)
1583+
#[test]
1584+
#[cfg(unix)]
1585+
fn test_stty_uses_stdin() {
1586+
use std::fs::File;
1587+
use std::process::Stdio;
1588+
1589+
let (path, _controller, _replica) = pty_path();
1590+
1591+
// Output redirection: stty > file (stdin is still TTY)
1592+
let stdin = File::open(&path).unwrap();
1593+
new_ucmd!()
1594+
.set_stdin(stdin)
1595+
.set_stdout(Stdio::piped())
1596+
.succeeds()
1597+
.stdout_contains("speed");
1598+
1599+
// Save/restore: stty $(stty -g) pattern
1600+
let stdin = File::open(&path).unwrap();
1601+
let saved = new_ucmd!()
1602+
.arg("-g")
1603+
.set_stdin(stdin)
1604+
.set_stdout(Stdio::piped())
1605+
.succeeds()
1606+
.stdout_str()
1607+
.trim()
1608+
.to_string();
1609+
assert!(saved.contains(':'), "Expected colon-separated saved state");
1610+
1611+
let stdin = File::open(&path).unwrap();
1612+
new_ucmd!().arg(&saved).set_stdin(stdin).succeeds();
1613+
1614+
// Stdin redirection: stty rows 30 cols 100 < /dev/pts/N
1615+
let stdin = File::open(&path).unwrap();
1616+
new_ucmd!()
1617+
.args(&["rows", "30", "cols", "100"])
1618+
.set_stdin(stdin)
1619+
.succeeds();
1620+
1621+
let stdin = File::open(&path).unwrap();
1622+
new_ucmd!()
1623+
.arg("--all")
1624+
.set_stdin(stdin)
1625+
.succeeds()
1626+
.stdout_contains("rows 30")
1627+
.stdout_contains("columns 100");
1628+
}
1629+
15601630
#[test]
15611631
#[cfg(unix)]
15621632
fn test_columns_env_wrapping() {

0 commit comments

Comments
 (0)