From 5fe6706c510945524942813159478527f2323568 Mon Sep 17 00:00:00 2001 From: Daringcuteseal Date: Sat, 4 Jan 2025 11:30:57 +0700 Subject: [PATCH 1/3] head: refactor to use specific error enums --- Cargo.lock | 1 + src/uu/head/Cargo.toml | 1 + src/uu/head/src/head.rs | 75 ++++++++++++++++++++++++++++------------- 3 files changed, 53 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bfca23683..e40171f46 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2770,6 +2770,7 @@ version = "0.0.28" dependencies = [ "clap", "memchr", + "thiserror 2.0.9", "uucore", ] diff --git a/src/uu/head/Cargo.toml b/src/uu/head/Cargo.toml index 5e2b977b8..1ccf3b985 100644 --- a/src/uu/head/Cargo.toml +++ b/src/uu/head/Cargo.toml @@ -19,6 +19,7 @@ path = "src/head.rs" [dependencies] clap = { workspace = true } memchr = { workspace = true } +thiserror = { workspace = true } uucore = { workspace = true, features = ["ringbuffer", "lines", "fs"] } [[bin]] diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index 04751fa71..70f9653f2 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -8,8 +8,10 @@ use clap::{crate_version, Arg, ArgAction, ArgMatches, Command}; use std::ffi::OsString; use std::io::{self, BufWriter, ErrorKind, Read, Seek, SeekFrom, Write}; +use std::num::TryFromIntError; +use thiserror::Error; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError}; +use uucore::error::{FromIo, UError, UResult}; use uucore::line_ending::LineEnding; use uucore::lines::lines; use uucore::{format_usage, help_about, help_usage, show}; @@ -37,6 +39,36 @@ mod take; use take::take_all_but; use take::take_lines; +#[derive(Error, Debug)] +enum HeadError { + /// Wrapper around `io::Error` + #[error("error reading {name}: {err}")] + Io { name: String, err: io::Error }, + + #[error("parse error: {0}")] + ParseError(String), + + #[error("bad argument encoding")] + BadEncoding, + + #[error("{0}: number of -bytes or -lines is too large")] + NumTooLarge(#[from] TryFromIntError), + + #[error("clap error: {0}")] + Clap(#[from] clap::Error), + + #[error("{0}")] + MatchOption(String), +} + +impl UError for HeadError { + fn code(&self) -> i32 { + 1 + } +} + +type HeadResult = Result; + pub fn uu_app() -> Command { Command::new(uucore::util_name()) .version(crate_version!()) @@ -152,7 +184,7 @@ impl Mode { fn arg_iterate<'a>( mut args: impl uucore::Args + 'a, -) -> UResult + 'a>> { +) -> HeadResult + 'a>> { // argv[0] is always present let first = args.next().unwrap(); if let Some(second) = args.next() { @@ -160,22 +192,19 @@ fn arg_iterate<'a>( match parse::parse_obsolete(s) { Some(Ok(iter)) => Ok(Box::new(vec![first].into_iter().chain(iter).chain(args))), Some(Err(e)) => match e { - parse::ParseError::Syntax => Err(USimpleError::new( - 1, - format!("bad argument format: {}", s.quote()), - )), - parse::ParseError::Overflow => Err(USimpleError::new( - 1, - format!( - "invalid argument: {} Value too large for defined datatype", - s.quote() - ), - )), + parse::ParseError::Syntax => Err(HeadError::ParseError(format!( + "bad argument format: {}", + s.quote() + ))), + parse::ParseError::Overflow => Err(HeadError::ParseError(format!( + "invalid argument: {} Value too large for defined datatype", + s.quote() + ))), }, None => Ok(Box::new(vec![first, second].into_iter().chain(args))), } } else { - Err(USimpleError::new(1, "bad argument encoding".to_owned())) + Err(HeadError::BadEncoding) } } else { Ok(Box::new(vec![first].into_iter())) @@ -247,10 +276,7 @@ fn catch_too_large_numbers_in_backwards_bytes_or_lines(n: u64) -> Option match usize::try_from(n) { Ok(value) => Some(value), Err(e) => { - show!(USimpleError::new( - 1, - format!("{e}: number of -bytes or -lines is too large") - )); + show!(HeadError::NumTooLarge(e)); None } } @@ -511,16 +537,17 @@ fn uu_head(options: &HeadOptions) -> UResult<()> { head_file(&mut file, options) } }; - if res.is_err() { + if let Err(e) = res { let name = if file.as_str() == "-" { "standard input" } else { file }; - show!(USimpleError::new( - 1, - format!("error reading {name}: Input/output error") - )); + return Err(HeadError::Io { + name: name.to_string(), + err: e, + } + .into()); } first = false; } @@ -537,7 +564,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = match HeadOptions::get_from(&matches) { Ok(o) => o, Err(s) => { - return Err(USimpleError::new(1, s)); + return Err(HeadError::MatchOption(s).into()); } }; uu_head(&args) From 9202f23787fdf7fdfdf8e2e603a5b1092b1b6133 Mon Sep 17 00:00:00 2001 From: Daringcuteseal Date: Sat, 4 Jan 2025 11:31:37 +0700 Subject: [PATCH 2/3] tests/head: add test to write to /dev/full --- tests/by-util/test_head.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index 1d4b15963..6d7ecffb2 100644 --- a/tests/by-util/test_head.rs +++ b/tests/by-util/test_head.rs @@ -475,3 +475,25 @@ fn test_all_but_last_lines() { .succeeds() .stdout_is_fixture("lorem_ipsum_backwards_15_lines.expected"); } + +#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))] +#[test] +fn test_write_to_dev_full() { + use std::fs::OpenOptions; + + for append in [true, false] { + { + let dev_full = OpenOptions::new() + .write(true) + .append(append) + .open("/dev/full") + .unwrap(); + + new_ucmd!() + .pipe_in_fixture(INPUT) + .set_stdout(dev_full) + .run() + .stderr_contains("No space left on device"); + } + } +} From 509b755b3b19e753cc7bf1d577dec4c7033c8b06 Mon Sep 17 00:00:00 2001 From: Daringcuteseal Date: Sat, 4 Jan 2025 11:33:30 +0700 Subject: [PATCH 3/3] head: make process fail when writing to /dev/full --- src/uu/head/src/head.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index 70f9653f2..52d52f13b 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -3,11 +3,11 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (vars) BUFWRITER seekable +// spell-checker:ignore (vars) seekable use clap::{crate_version, Arg, ArgAction, ArgMatches, Command}; use std::ffi::OsString; -use std::io::{self, BufWriter, ErrorKind, Read, Seek, SeekFrom, Write}; +use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; use std::num::TryFromIntError; use thiserror::Error; use uucore::display::Quotable; @@ -18,9 +18,6 @@ use uucore::{format_usage, help_about, help_usage, show}; const BUF_SIZE: usize = 65536; -/// The capacity in bytes for buffered writers. -const BUFWRITER_CAPACITY: usize = 16_384; // 16 kilobytes - const ABOUT: &str = help_about!("head.md"); const USAGE: &str = help_usage!("head.md"); @@ -255,6 +252,11 @@ where io::copy(&mut reader, &mut stdout)?; + // Make sure we finish writing everything to the target before + // exiting. Otherwise, when Rust is implicitly flushing, any + // error will be silently ignored. + stdout.flush()?; + Ok(()) } @@ -263,11 +265,14 @@ fn read_n_lines(input: &mut impl std::io::BufRead, n: u64, separator: u8) -> std let mut reader = take_lines(input, n, separator); // Write those bytes to `stdout`. - let stdout = std::io::stdout(); - let stdout = stdout.lock(); - let mut writer = BufWriter::with_capacity(BUFWRITER_CAPACITY, stdout); + let mut stdout = std::io::stdout(); - io::copy(&mut reader, &mut writer)?; + io::copy(&mut reader, &mut stdout)?; + + // Make sure we finish writing everything to the target before + // exiting. Otherwise, when Rust is implicitly flushing, any + // error will be silently ignored. + stdout.flush()?; Ok(()) }