From ed612eaca68ce6c7d25856099d50313a16bc5afd Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 04:31:25 +0300 Subject: [PATCH] tail: report a read/seek failure instead of panicking Once `bounded_tail` decides a file is seekable, every seek and read it does to find the start of the last N lines/bytes assumed success and unwrapped the result. A seekable file that then fails to read -- a character device that times out or errors mid-read, for instance -- crashed instead of reporting the failure: $ tail /dev/drm_dp_aux2 thread 'main' panicked at src/uu/tail/src/chunks.rs:92:40: called `Result::unwrap()` on an `Err` value: Os { code: 5, ... } GNU reports it as a read error and exits 1: $ tail /dev/drm_dp_aux2 tail: error reading '/dev/drm_dp_aux2': Input/output error `ReverseChunks` (used by `-n`'s negative case) and every seek in `bounded_tail` (used by all of `-n`/`-c`, positive and negative) now return `io::Result` instead of unwrapping, converted at the one call site into the same message `tail` already uses for an unreadable directory. Verified against the same device as root, across every `bounded_tail` mode (default, `-n N`, `-n +N`, `-c N`, `-c +N`): each now matches GNU's message and exit code exactly, where each previously panicked (the `-c` cases already avoided a panic before this change, by going through a different, already-fallible copy path, but reported a bare "Input/output error" without the filename; that gap is unchanged here and is a separate, pre-existing issue). No portable, root-free way to reproduce the underlying condition in the test suite -- it needs a real device whose read genuinely fails after open -- so this is verified manually rather than by an added test; cargo test --features tail --test tests -- test_tail (161 pre-existing, 0 new) is unaffected. --- src/uu/tail/src/chunks.rs | 31 +++++++++---------- src/uu/tail/src/tail.rs | 64 +++++++++++++++++++++++++-------------- 2 files changed, 57 insertions(+), 38 deletions(-) diff --git a/src/uu/tail/src/chunks.rs b/src/uu/tail/src/chunks.rs index acf6b0a286..3666b6fdfe 100644 --- a/src/uu/tail/src/chunks.rs +++ b/src/uu/tail/src/chunks.rs @@ -12,7 +12,7 @@ use std::collections::VecDeque; use std::fs::File; -use std::io::{BufRead, Read, Seek, SeekFrom, Write}; +use std::io::{self, BufRead, Read, Seek, SeekFrom, Write}; use uucore::error::UResult; /// When reading files in reverse in `bounded_tail`, this is the size of each @@ -46,26 +46,26 @@ pub struct ReverseChunks<'a> { } impl<'a> ReverseChunks<'a> { - pub fn new(file: &'a mut File) -> Self { + pub fn new(file: &'a mut File) -> io::Result { let current = if cfg!(unix) { - file.stream_position().unwrap() + file.stream_position()? } else { 0 }; - let size = file.seek(SeekFrom::End(0)).unwrap() - current; + let size = file.seek(SeekFrom::End(0))? - current; let max_blocks_to_read = (size as f64 / BLOCK_SIZE as f64).ceil() as usize; let block_idx = 0; - ReverseChunks { + Ok(ReverseChunks { file, size, max_blocks_to_read, block_idx, - } + }) } } impl Iterator for ReverseChunks<'_> { - type Item = Vec; + type Item = io::Result>; fn next(&mut self) -> Option { // If there are no more chunks to read, terminate the iterator. @@ -85,20 +85,19 @@ impl Iterator for ReverseChunks<'_> { // Seek backwards by the next chunk, read the full chunk into // `buf`, and then seek back to the start of the chunk again. let mut buf = vec![0; block_size as usize]; - let pos = self - .file - .seek(SeekFrom::Current(-(block_size as i64))) - .unwrap(); - self.file.read_exact(&mut buf).unwrap(); - let pos2 = self + let result = self .file .seek(SeekFrom::Current(-(block_size as i64))) - .unwrap(); - assert_eq!(pos, pos2); + .and_then(|pos| { + self.file.read_exact(&mut buf)?; + let pos2 = self.file.seek(SeekFrom::Current(-(block_size as i64)))?; + assert_eq!(pos, pos2); + Ok(buf) + }); self.block_idx += 1; - Some(buf) + Some(result) } } diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index b6da02264e..81513ed116 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -32,7 +32,7 @@ use std::fs::File; use std::io::{self, BufReader, BufWriter, ErrorKind, Read, Seek, SeekFrom, Write, stdin, stdout}; use std::path::{Path, PathBuf}; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError, set_exit_code}; +use uucore::error::{FromIo, UError, UResult, USimpleError, set_exit_code, strip_errno}; use uucore::translate; use uucore::{show, show_error}; @@ -178,7 +178,7 @@ fn tail_file( && file.is_seekable(if input.is_stdin() { offset } else { 0 }) && (!st.is_file() || st.len() > blksize_limit) { - bounded_tail(&mut file, settings)?; + bounded_tail(&mut file, settings, input.display_name.as_str())?; reader = BufReader::new(file); } else { reader = BufReader::new(file); @@ -406,16 +406,17 @@ fn forwards_thru_file( /// Iterate over bytes in the file, in reverse, until we find the /// `num_delimiters` instance of `delimiter`. The `file` is left seek'd to the /// position just after that delimiter. -fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) { +fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) -> io::Result<()> { if num_delimiters == 0 { - file.seek(SeekFrom::End(0)).unwrap(); - return; + file.seek(SeekFrom::End(0))?; + return Ok(()); } // This variable counts the number of delimiters found in the file // so far (reading from the end of the file toward the beginning). let mut counter = 0; let mut first_slice = true; - for slice in ReverseChunks::new(file) { + for slice in ReverseChunks::new(file)? { + let slice = slice?; // Iterate over each byte in the slice in reverse order. let mut iter = memrchr_iter(delimiter, &slice); @@ -440,37 +441,34 @@ fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) { // cursor in the file is at the *beginning* of the // block, so seeking forward by `i + 1` bytes puts // us right after the found delimiter. - file.seek(SeekFrom::Current((i + 1) as i64)).unwrap(); - return; + file.seek(SeekFrom::Current((i + 1) as i64))?; + return Ok(()); } } } + Ok(()) } -/// When tail'ing a file, we do not need to read the whole file from start to -/// finish just to find the last n lines or bytes. Instead, we can seek to the -/// end of the file, and then read the file "backwards" in blocks of size -/// `BLOCK_SIZE` until we find the location of the first line/byte. This ends up -/// being a nice performance win for very large files. -fn bounded_tail(file: &mut File, settings: &Settings) -> UResult<()> { - debug_assert!(!settings.presume_input_pipe); +/// Seek `file` to where `bounded_tail` should start reading from, returning +/// the byte limit for [`FilterMode::Bytes`]' negative case, if any. +fn seek_to_tail_start(file: &mut File, settings: &Settings) -> io::Result> { let mut limit = None; // Find the position in the file to start printing from. match &settings.mode { FilterMode::Lines(Signum::Negative(count), delimiter) => { - backwards_thru_file(file, *count, *delimiter); + backwards_thru_file(file, *count, *delimiter)?; } FilterMode::Lines(Signum::Positive(count), delimiter) if count > &1 => { - let i = forwards_thru_file(file, *count - 1, *delimiter).unwrap(); - file.seek(SeekFrom::Start(i as u64)).unwrap(); + let i = forwards_thru_file(file, *count - 1, *delimiter)?; + file.seek(SeekFrom::Start(i as u64))?; } FilterMode::Lines(Signum::MinusZero, _) | FilterMode::Bytes(Signum::MinusZero) => { - file.seek(SeekFrom::End(0)).unwrap(); + file.seek(SeekFrom::End(0))?; } FilterMode::Bytes(Signum::Negative(count)) => { if file.seek(SeekFrom::End(-(*count as i64))).is_err() { - file.seek(SeekFrom::Start(0)).unwrap(); + file.seek(SeekFrom::Start(0))?; } limit = Some(*count); } @@ -481,12 +479,34 @@ fn bounded_tail(file: &mut File, settings: &Settings) -> UResult<()> { // underlying `lseek` fail with `EINVAL`; treat that like a start // beyond the end of the file and produce no output. file.seek(SeekFrom::Start(*count - 1)) - .or_else(|_| file.seek(SeekFrom::End(0))) - .unwrap(); + .or_else(|_| file.seek(SeekFrom::End(0)))?; } _ => {} } + Ok(limit) +} + +/// The message GNU gives for any read/seek failure while tailing a +/// (previously successfully opened) file, keyed to the name as displayed +/// rather than the raw error, which is quoted `strip_errno`'s way. +fn tail_io_error(display_name: &str, error: io::Error) -> Box { + USimpleError::new( + 1, + translate!("tail-error-reading-file", "file" => display_name.to_owned(), "error" => strip_errno(&error)), + ) +} + +/// When tail'ing a file, we do not need to read the whole file from start to +/// finish just to find the last n lines or bytes. Instead, we can seek to the +/// end of the file, and then read the file "backwards" in blocks of size +/// `BLOCK_SIZE` until we find the location of the first line/byte. This ends up +/// being a nice performance win for very large files. +fn bounded_tail(file: &mut File, settings: &Settings, display_name: &str) -> UResult<()> { + debug_assert!(!settings.presume_input_pipe); + + let limit = seek_to_tail_start(file, settings).map_err(|e| tail_io_error(display_name, e))?; + print_target_section(file, limit)?; Ok(()) }