diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 0600c4e98ea..665706a5e79 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -22,7 +22,7 @@ use clap::{Arg, ArgAction, ArgMatches, Command, parser::ValueSource}; use std::ffi::OsString; use std::io::{BufWriter, Write, stdout}; -use std::path::{Path, PathBuf}; +use std::path::Path; use thiserror::Error; use crate::blocks::{BlockSize, read_block_size}; @@ -416,10 +416,11 @@ impl UError for DfError { } /// Gather the filesystems that `df` would report on, without formatting anything. -pub fn filesystems

(paths: Option<&[P]>, opt: &Options) -> UResult> -where - P: AsRef, -{ +/// +/// `paths` is `None` to report on every mounted filesystem, as `df` does when +/// called without operands, or `Some` to report on the filesystems containing +/// the given paths. +pub fn filesystems(paths: Option<&[&Path]>, opt: &Options) -> UResult> { // Run a sync call before any operation if so instructed. if opt.sync { #[cfg(not(any(windows, target_os = "redox")))] @@ -458,10 +459,9 @@ where } /// Display filesystem usage information as a table on stdout. -pub fn df

(paths: Option<&[P]>, opt: &Options) -> UResult<()> -where - P: AsRef, -{ +/// +/// `paths` has the same meaning as in [`filesystems`]. +pub fn df(paths: Option<&[&Path]>, opt: &Options) -> UResult<()> { let filesystems = filesystems(paths, opt)?; // Every path given on the command line was rejected; `filesystems` has @@ -495,9 +495,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } let opt = Options::from_matches(&matches)?; - let paths: Option> = matches + let paths: Option> = matches .get_many::(OPT_PATHS) - .map(|paths| paths.map(PathBuf::from).collect()); + .map(|paths| paths.map(Path::new).collect()); df(paths.as_deref(), &opt) } @@ -656,10 +656,20 @@ mod tests { // spell-checker:ignore apfs mod write_table { + use crate::blocks::BlockSize; use crate::{Filesystem, Options, write_table}; use std::ffi::OsString; use uucore::fsext::{FsUsage, MountInfo}; + /// `Options::default()` picks up the block size from the environment + /// (`POSIXLY_CORRECT` halves it), so pin it for reproducible output. + fn options() -> Options { + Options { + block_size: BlockSize::Bytes(1024), + ..Options::default() + } + } + fn filesystem() -> Filesystem { Filesystem { file: Some(OsString::from("/tmp/file")), @@ -690,7 +700,7 @@ mod tests { #[test] fn test_write_table_to_arbitrary_writer() { let mut buffer: Vec = Vec::new(); - write_table(&mut buffer, vec![filesystem()], &Options::default()).unwrap(); + write_table(&mut buffer, vec![filesystem()], &options()).unwrap(); // Header text is localized and the bundle is not loaded in unit // tests, so only the data row is asserted on here. @@ -710,7 +720,7 @@ mod tests { #[test] fn test_write_table_empty() { let mut buffer: Vec = Vec::new(); - write_table(&mut buffer, vec![], &Options::default()).unwrap(); + write_table(&mut buffer, vec![], &options()).unwrap(); let table = String::from_utf8(buffer).unwrap(); assert_eq!(table.lines().count(), 1); diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index e8ab1a8f506..0bc3a8a85d0 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -26,7 +26,7 @@ use std::io::{BufRead, BufReader, ErrorKind, Read, Seek, SeekFrom, Write, stdin} use std::path::Path; use thiserror::Error; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; +use uucore::error::{FromIo, UResult, USimpleError, UUsageError, set_exit_code, strip_errno}; use uucore::parser::parse_size::parse_size_u64; use uucore::translate; @@ -521,6 +521,12 @@ struct ByteChunkWriter<'a> { /// Iterator that yields filenames for each chunk. filename_iterator: FilenameIterator<'a>, + + /// Current filename being written to. + current_filename: OsString, + + /// Whether an error has already been reported for the current file. + error_reported: bool, } impl<'a> ByteChunkWriter<'a> { @@ -540,8 +546,45 @@ impl<'a> ByteChunkWriter<'a> { num_chunks_written: 0, inner, filename_iterator, + current_filename: filename, + error_reported: false, }) } + + /// Report a write error on the current chunk, at most once per chunk. + /// + /// Returns an empty error: the message is already out, so the caller only + /// has to stop, without printing anything else. + fn report_error(&mut self, e: &io::Error) -> io::Error { + if !self.error_reported { + uucore::show_error!("{}: {}", self.current_filename.display(), strip_errno(e)); + set_exit_code(1); + self.error_reported = true; + } + io::Error::other("") + } + + /// Write to the current chunk, flushing right away. + /// + /// Without the flush, a write failure on a full device would only surface + /// when the buffer happens to be flushed, by which point we have moved on + /// to another chunk and would blame the wrong file. + fn write_chunk(&mut self, bytes: &[u8]) -> io::Result { + let written = custom_write(bytes, &mut self.inner, self.settings) + .and_then(|n| self.inner.flush().map(|()| n)); + match written { + Err(e) if ignorable_io_error(&e, self.settings) => Ok(bytes.len()), + Err(e) => Err(self.report_error(&e)), + ok => ok, + } + } +} + +impl Drop for ByteChunkWriter<'_> { + fn drop(&mut self) { + // Safety net: a buffered error must not be lost when the writer goes away. + let _ = self.inner.flush().map_err(|e| self.report_error(&e)); + } } impl Write for ByteChunkWriter<'_> { @@ -555,6 +598,10 @@ impl Write for ByteChunkWriter<'_> { let mut carryover_bytes_written: usize = 0; while !buf.is_empty() { if self.num_bytes_remaining_in_current_chunk == 0 { + // Flush before switching file, so a delayed write error is + // still attributed to the chunk it belongs to. + self.inner.flush().map_err(|e| self.report_error(&e))?; + // Increment the chunk number, reset the number of bytes remaining, and instantiate the new underlying writer. self.num_chunks_written += 1; self.num_bytes_remaining_in_current_chunk = self.chunk_size; @@ -567,6 +614,8 @@ impl Write for ByteChunkWriter<'_> { writeln!(io::stdout(), "creating file {}", filename.quote())?; } self.inner = self.settings.instantiate_current_writer(&filename, true)?; + self.current_filename = filename; + self.error_reported = false; } // If the capacity of this chunk is greater than the number of @@ -575,7 +624,7 @@ impl Write for ByteChunkWriter<'_> { // the chunk number and repeat. let buf_len = buf.len(); if (buf_len as u64) < self.num_bytes_remaining_in_current_chunk { - let num_bytes_written = custom_write(buf, &mut self.inner, self.settings)?; + let num_bytes_written = self.write_chunk(buf)?; self.num_bytes_remaining_in_current_chunk -= num_bytes_written as u64; return Ok(carryover_bytes_written + num_bytes_written); } @@ -586,7 +635,7 @@ impl Write for ByteChunkWriter<'_> { // self.num_bytes_remaining_in_current_chunk is lower than // n, which is already usize. let i = self.num_bytes_remaining_in_current_chunk as usize; - let num_bytes_written = custom_write(&buf[..i], &mut self.inner, self.settings)?; + let num_bytes_written = self.write_chunk(&buf[..i])?; self.num_bytes_remaining_in_current_chunk -= num_bytes_written as u64; // It's possible that the underlying writer did not @@ -604,7 +653,7 @@ impl Write for ByteChunkWriter<'_> { Ok(carryover_bytes_written) } fn flush(&mut self) -> io::Result<()> { - self.inner.flush() + self.inner.flush().map_err(|e| self.report_error(&e)) } } @@ -1359,8 +1408,15 @@ fn split(settings: &Settings) -> UResult<()> { Strategy::Bytes(chunk_size) => { let mut writer = ByteChunkWriter::new(chunk_size, settings)?; // todo: distinct read error and write error - io::copy(&mut reader, &mut writer)?; - Ok(()) + match io::copy(&mut reader, &mut writer) { + Ok(_) => Ok(()), + // The writer already reported the write error and set the exit + // code, and signals that with an empty error: stay quiet here. + Err(e) if e.kind() == ErrorKind::Other && e.to_string().is_empty() => { + Err(USimpleError::new(1, "")) + } + Err(e) => Err(e.into()), + } } Strategy::LineBytes(chunk_size) => { line_bytes(settings, &mut reader, chunk_size as usize, io_blksize) diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index a9e36f2c5a5..d53f66f05c4 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -2115,3 +2115,26 @@ fn test_io_error() { //todo: add file path with proper distinction of input/output .stderr_contains("Input/output error\n"); } + +/// Writing a chunk to a full device must be reported and must stop the split. +#[test] +#[cfg(target_os = "linux")] +fn test_write_error_on_full_device() { + if !Path::new("/dev/full").exists() { + return; + } + + let (at, mut ucmd) = at_and_ucmd!(); + + // The first chunk lands on /dev/full, so its write can never succeed. + at.symlink_file("/dev/full", "xaa"); + at.write("input", "uv"); + + ucmd.args(&["-b", "1", "input"]) + .fails_with_code(1) + .no_stdout() + .stderr_contains("split: xaa: No space left on device"); + + // split must not have moved on to the next chunk. + assert!(!at.file_exists("xab")); +}