Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .vscode/cspell.dictionaries/jargon.wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ unportable
unsync
unwritable
urand
ustdin
utimensat
whitespace
wordlist
Expand Down
2 changes: 1 addition & 1 deletion src/uu/base32/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ doctest = false
[dependencies]
clap = { workspace = true }
fluent = { workspace = true }
uucore = { workspace = true, features = ["encoding"] }
uucore = { workspace = true, features = ["encoding", "stdio"] }

[target.'cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))'.dependencies]
rustix = { workspace = true, features = ["fs"] }
Expand Down
3 changes: 2 additions & 1 deletion src/uu/base32/src/base_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use uucore::encoding::{
};
use uucore::error::{FromIo, UResult, USimpleError, UUsageError, strip_errno};
use uucore::format_usage;
use uucore::stdio::ustdin;
use uucore::translate;

pub const BASE_CMD_PARSE_ERROR: i32 = 1;
Expand Down Expand Up @@ -151,7 +152,7 @@ pub fn get_input(config: &Config) -> UResult<Box<dyn BufRead>> {
// Stdin is already buffered by the OS; wrap once more to reduce syscalls per read.
Ok(Box::new(BufReader::with_capacity(
DEFAULT_BUF_SIZE,
io::stdin(),
ustdin(),
)))
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/uu/csplit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ clap = { workspace = true }
fluent = { workspace = true }
regex = { workspace = true }
thiserror = { workspace = true }
uucore = { workspace = true, features = ["entries", "fs", "format"] }
uucore = { workspace = true, features = ["entries", "fs", "format", "stdio"] }

[[bin]]
name = "csplit"
Expand Down
48 changes: 25 additions & 23 deletions src/uu/csplit/src/csplit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@

use std::borrow::Borrow;
use std::ffi::OsString;
use std::io::{self, BufReader};
use std::{
fs::{File, remove_file},
io::{BufRead, BufWriter, Write},
};
use std::fs::{File, remove_file};

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.

Going forward, it would be better to keep unrelated style changes in a separate commit.

use std::io;
use std::io::{BufRead, BufReader, BufWriter, Write};

use clap::{Arg, ArgAction, ArgMatches, Command};
use regex::bytes::Regex;
use uucore::display::Quotable;
use uucore::error::{FromIo, UResult};
use uucore::format_usage;
use uucore::stdio::ustdin;
use uucore::{format_usage, show};

mod csplit_error;
mod diagnostics;
Expand Down Expand Up @@ -127,10 +126,8 @@ where
| patterns::Pattern::SkipToMatch(_, _, patterns::ExecutePattern::Always)
)
});
let ret = do_csplit(&mut split_writer, patterns_vec, &mut input_iter);

// consume the rest, unless there was an error
let ret = if ret.is_ok() {
let ret = do_csplit(&mut split_writer, patterns_vec, &mut input_iter).and_then(|_| {
// consume the rest, unless there was an error
input_iter.rewind_buffer();
if let Some((_, line)) = input_iter.next() {
// There is remaining input: create a final split and copy remainder
Expand All @@ -148,20 +145,25 @@ where
} else {
Ok(())
}
} else {
ret
};
// GNU still reports the bytes written to the split that was in progress
// when the failure happened.
if ret.is_err() && !split_writer.split_finished {
let _ = split_writer.finish_split();
}
});

// delete files on error by default
if ret.is_err() && !options.keep_files {
split_writer.delete_all_splits()?;
if let Err(e) = ret {
// Show error first
show!(e);

// GNU still reports the bytes written to the split that was in progress
// when the failure happened.
if !split_writer.split_finished {
let _ = split_writer.finish_split();
}

// delete files on error by default
if !options.keep_files {
split_writer.delete_all_splits()?;
}
}
ret

Ok(())
}

fn do_csplit<I>(
Expand Down Expand Up @@ -669,7 +671,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {

let options = CsplitOptions::new(&matches).map_err(report)?;
if file_name == "-" {
let stdin = io::stdin();
let stdin = ustdin();
csplit(&options, &patterns, stdin.lock()).map_err(report)?;
} else {
let file = File::open(file_name)
Expand Down
10 changes: 9 additions & 1 deletion src/uucore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,14 @@ default = ["signals"]
backup-control = []
colors = []
diagnostics = ["dep:ariadne"]
checksum = ["base64-simd", "quoting-style", "rustix/fs", "sum"]
checksum = [
"base64-simd",
"quoting-style",
"rustix/fs",
"signals",
"stdio",
"sum",
]
encoding = ["data-encoding", "data-encoding-macro", "z85", "base64-simd"]
entries = ["libc", "rustix/fs", "rustix/process"]
extendedbigdecimal = ["bigdecimal", "num-traits"]
Expand Down Expand Up @@ -203,6 +210,7 @@ safe-traversal = ["libc", "nix/fs", "nix/dir", "nix/user", "rustix/fs"]
selinux = ["dep:selinux", "libc"]
smack = ["xattr"]
signals = ["nix/fs", "nix/signal", "nix/poll"]
stdio = []

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.

Suggested change
stdio = []
stdio = ["signals"]

we should enable the signals feature here, and not directly in the checksum feature

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not sure about that. I made the stdio feature be usable without signals, in case we ever need to further extend the capacities of the wrapper. The idea is to be able to use ustdin() everywhere in the uutils, no matter the availability of signals

sum = [
"digest",
"hex",
Expand Down
1 change: 1 addition & 0 deletions src/uucore/locales/en-US.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ error-invalid-argument = Invalid argument
error-is-a-directory-text = Is a directory
error-is-a-directory = { $file }: { error-is-a-directory-text }
error-too-many-symlink-levels = Too many levels of symbolic links
error-bad-file-descriptor = Bad file descriptor
# Common actions
action-copying = copying
Expand Down
1 change: 1 addition & 0 deletions src/uucore/locales/fr-FR.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ error-invalid-argument = Argument invalide
error-is-a-directory-text = Est un répertoire
error-is-a-directory = { $file }: { error-is-a-directory-text }
error-too-many-symlink-levels = Trop de niveaux de liens symboliques
error-bad-file-descriptor = Mauvais descripteur de fichier
# Actions communes
action-copying = copie
Expand Down
2 changes: 2 additions & 0 deletions src/uucore/src/lib/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ pub mod selinux;
pub mod signals;
#[cfg(all(feature = "smack", target_os = "linux"))]
pub mod smack;
#[cfg(feature = "stdio")]
pub mod stdio;
#[cfg(all(feature = "systemd_logind", target_os = "linux"))]
pub mod systemd_logind;
#[cfg(all(
Expand Down
3 changes: 2 additions & 1 deletion src/uucore/src/lib/features/checksum/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::checksum::{
use crate::error::{FromIo, UResult, USimpleError};
use crate::line_ending::LineEnding;
use crate::quoting_style::locale_aware_shell_escape;
use crate::stdio::ustdin;
use crate::sum::DigestOutput;
use crate::{show, translate_text};

Expand Down Expand Up @@ -245,7 +246,7 @@ where
let mut file = io::BufReader::with_capacity(
READ_BUFFER_SIZE,
if filename == "-" {
stdin_buf = io::stdin();
stdin_buf = ustdin();
Box::new(stdin_buf) as Box<dyn io::Read>
} else {
let filepath = Path::new(filename);
Expand Down
94 changes: 94 additions & 0 deletions src/uucore/src/lib/features/stdio.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
use std::io;

#[cfg(all(feature = "signals", unix, not(target_os = "fuchsia")))]
use crate::{signals, translate};
Comment thread
RenjiSann marked this conversation as resolved.
Comment thread
RenjiSann marked this conversation as resolved.

#[cfg_attr(
not(all(feature = "signals", unix, not(target_os = "fuchsia"))),
expect(clippy::unnecessary_wraps)
)]
fn err_if_stdin_closed() -> io::Result<()> {
#[cfg(all(feature = "signals", unix, not(target_os = "fuchsia")))]
if signals::stdin_was_closed() {
return Err(io::Error::other(translate!("error-bad-file-descriptor")));
}

Ok(())
}

/// Wrapper around [`std::io::Stdin`] that checks if stdin was closed
/// before trying to read.
pub struct UStdin {
inner: io::Stdin,
}

impl UStdin {
pub fn lock(&self) -> UStdinLock<'static> {
UStdinLock {
inner: self.inner.lock(),
}
}
}

impl io::Read for UStdin {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
err_if_stdin_closed()?;
self.inner.read(buf)
}

fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
err_if_stdin_closed()?;
self.inner.read_to_end(buf)
}

fn read_to_string(&mut self, buf: &mut String) -> io::Result<usize> {
Comment thread
RenjiSann marked this conversation as resolved.
err_if_stdin_closed()?;
self.inner.read_to_string(buf)
}

fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
err_if_stdin_closed()?;
self.inner.read_exact(buf)
}
}

pub struct UStdinLock<'a> {
inner: io::StdinLock<'a>,
}

impl io::Read for UStdinLock<'_> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
err_if_stdin_closed()?;
self.inner.read(buf)
}

fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
err_if_stdin_closed()?;
self.inner.read_to_end(buf)
}

fn read_to_string(&mut self, buf: &mut String) -> io::Result<usize> {
err_if_stdin_closed()?;
self.inner.read_to_string(buf)
}

fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
err_if_stdin_closed()?;
self.inner.read_exact(buf)
}
}

impl io::BufRead for UStdinLock<'_> {
fn fill_buf(&mut self) -> io::Result<&[u8]> {
err_if_stdin_closed()?;
self.inner.fill_buf()
}

fn consume(&mut self, amount: usize) {
self.inner.consume(amount);
}
}

pub fn ustdin() -> UStdin {

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.

I'd probably just name this stdin, and likewise for UStdin

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

why not follow the naming of other wrappers of uucore (like UResult, UError, etc...) ?

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.

why not follow the naming of other wrappers of uucore (like UResult, UError, etc...) ?

I’m not particularly fond of the naming of those types either, but they’re very common return types, so disambiguation may be necessary. That doesn’t apply in this case, though.

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.

The name ustdin at least should be stdin.

UStdin { inner: io::stdin() }
}
2 changes: 2 additions & 0 deletions src/uucore/src/lib/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ pub use crate::features::safe_traversal;
)
))]
pub use crate::features::signals;
#[cfg(feature = "stdio")]
pub use crate::features::stdio;
#[cfg(all(
feature = "utmpx",
any(
Expand Down
10 changes: 10 additions & 0 deletions tests/by-util/test_base32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,13 @@ fn test_base32_file_with_trailing_slash() {
.fails()
.stderr_only("base32: a/: Not a directory\n");
}

#[test]
#[cfg(unix)]
#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")]
fn test_closed_stdin() {
new_ucmd!()
.close_stdin(true)
.fails_with_code(1)
.stderr_is("base32: read error: Bad file descriptor\n");
}
10 changes: 10 additions & 0 deletions tests/by-util/test_base64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,3 +308,13 @@ fn test_base64_file_with_trailing_slash() {
.fails()
.stderr_only("base64: a/: Not a directory\n");
}

#[test]
#[cfg(unix)]
#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")]
fn test_closed_stdin() {
new_ucmd!()
.close_stdin(true)
.fails_with_code(1)
.stderr_is("base64: read error: Bad file descriptor\n");
}
10 changes: 10 additions & 0 deletions tests/by-util/test_cksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3920,3 +3920,13 @@ fn test_numeric_looking_directory_names_are_kept_verbatim() {
.fails_with_code(1)
.stderr_is("cksum: 007: Is a directory\ncksum: 1e3: Is a directory\n");
}

#[test]
#[cfg(unix)]
#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")]
fn test_closed_stdin() {
new_ucmd!()
.close_stdin(true)
.fails_with_code(1)
.stderr_is("cksum: -: Bad file descriptor\n");
}
12 changes: 12 additions & 0 deletions tests/by-util/test_csplit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1915,3 +1915,15 @@ fn test_empty_skip_to_regex_is_accepted() {
ucmd.args(&["letters", "%%"]).succeeds().stdout_only("19\n");
assert_eq!(at.read("xx00"), "delta\necho\nfoxtrot\n");
}

#[test]
#[cfg(unix)]
#[cfg_attr(wasi_runner, ignore = "WASI sandbox: Can't detect closed stdin")]
fn test_closed_stdin() {
new_ucmd!()
.args(&["-", "2", "4"])
.close_stdin(true)
.stderr_to_stdout()
.fails_with_code(1)
.stdout_is("csplit: read error: Bad file descriptor\n0\n");
}
22 changes: 22 additions & 0 deletions tests/uutests/src/lib/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1559,6 +1559,8 @@ pub struct UCommand {
has_run: bool,
ignore_stdin_write_error: bool,
stdin: Option<Stdio>,
#[cfg(unix)]
close_stdin: bool,
stdout: Option<Stdio>,
stderr: Option<Stdio>,
bytes_into_stdin: Option<Vec<u8>>,
Expand Down Expand Up @@ -1651,6 +1653,12 @@ impl UCommand {
self
}

#[cfg(unix)]
pub fn close_stdin(&mut self, close: bool) -> &mut Self {
self.close_stdin = close;
self
}

pub fn set_stdout<T: Into<Stdio>>(&mut self, stdout: T) -> &mut Self {
self.stdout = Some(stdout.into());
self
Expand Down Expand Up @@ -2004,6 +2012,20 @@ impl UCommand {
let mut stdin_pty: Option<File> = None;
#[cfg(not(all(unix, not(target_os = "redox"))))]
let stdin_pty: Option<File> = None;

#[cfg(unix)]
if self.close_stdin {
unsafe {
command.pre_exec(|| {
if libc::close(0) == -1 {
Err(io::Error::last_os_error())
} else {
Ok(())
}
})
};
}

if self.stderr_to_stdout {
let mut output = CapturedOutput::default();

Expand Down
Loading