Skip to content

Commit 61478bc

Browse files
committed
stty: use stdin for TTY operations instead of /dev/tty
1 parent dd08296 commit 61478bc

File tree

2 files changed

+44
-37
lines changed

2 files changed

+44
-37
lines changed

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: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,6 +1557,16 @@ 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+
new_ucmd!()
1565+
.pipe_in("")
1566+
.fails()
1567+
.stderr_contains("standard input: Inappropriate ioctl for device");
1568+
}
1569+
15601570
#[test]
15611571
#[cfg(unix)]
15621572
fn test_columns_env_wrapping() {

0 commit comments

Comments
 (0)