mirror of
https://github.com/uutils/coreutils.git
synced 2026-06-10 15:48:22 -07:00
split: fix I/O error handling for device full conditions
Should fix tests/split/split-io-err.sh
This commit is contained in:
@@ -38,6 +38,10 @@ split-error-cannot-determine-input-size = { $input }: cannot determine input siz
|
||||
split-error-cannot-determine-file-size = { $input }: cannot determine file size
|
||||
split-error-cannot-read-from-input = { $input }: cannot read from input : { $error }
|
||||
split-error-input-output-error = input/output error
|
||||
split-error-flush-before-file-switch = flush error before file switch
|
||||
split-error-flush-already-reported = flush error already reported
|
||||
split-error-write-already-reported = write error already reported
|
||||
split-error-flush-in-chunk-writer = flush error in ByteChunkWriter
|
||||
split-error-unable-to-open-file = unable to open { $file }; aborting
|
||||
split-error-unable-to-reopen-file = unable to re-open { $file }; aborting
|
||||
split-error-file-descriptor-limit = at file descriptor limit, but no file descriptor left to close. Closed { $count } writers before.
|
||||
|
||||
@@ -38,11 +38,16 @@ split-error-cannot-determine-input-size = { $input } : impossible de déterminer
|
||||
split-error-cannot-determine-file-size = { $input } : impossible de déterminer la taille du fichier
|
||||
split-error-cannot-read-from-input = { $input } : impossible de lire depuis l'entrée : { $error }
|
||||
split-error-input-output-error = erreur d'entrée/sortie
|
||||
split-error-flush-before-file-switch = erreur de vidage avant changement de fichier
|
||||
split-error-flush-already-reported = erreur de vidage déjà signalée
|
||||
split-error-write-already-reported = erreur d'écriture déjà signalée
|
||||
split-error-flush-in-chunk-writer = erreur de vidage dans ByteChunkWriter
|
||||
split-error-unable-to-open-file = impossible d'ouvrir { $file } ; abandon
|
||||
split-error-unable-to-reopen-file = impossible de rouvrir { $file } ; abandon
|
||||
split-error-file-descriptor-limit = limite de descripteurs de fichiers atteinte, mais aucun descripteur de fichier à fermer. { $count } écrivains fermés auparavant.
|
||||
split-error-shell-process-returned = Le processus shell a retourné { $code }
|
||||
split-error-shell-process-terminated = Le processus shell a été terminé par un signal
|
||||
split-error-is-a-directory = { $dir } : Est un répertoire
|
||||
|
||||
# Messages d'aide pour les options de ligne de commande
|
||||
split-help-bytes = mettre TAILLE octets par fichier de sortie
|
||||
|
||||
@@ -143,19 +143,27 @@ pub fn instantiate_current_writer(
|
||||
ErrorKind::IsADirectory => Error::other(
|
||||
translate!("split-error-is-a-directory", "dir" => filename),
|
||||
),
|
||||
_ => Error::other(
|
||||
ErrorKind::PermissionDenied => Error::other(
|
||||
translate!("split-error-unable-to-open-file", "file" => filename),
|
||||
),
|
||||
_ => Error::other(format!(
|
||||
"split: {filename}: {}",
|
||||
uucore::error::strip_errno(&e)
|
||||
)),
|
||||
})?
|
||||
} else {
|
||||
// re-open file that we previously created to append to it
|
||||
std::fs::OpenOptions::new()
|
||||
.append(true)
|
||||
.open(Path::new(&filename))
|
||||
.map_err(|_| {
|
||||
Error::other(
|
||||
.map_err(|e| match e.kind() {
|
||||
ErrorKind::PermissionDenied => Error::other(
|
||||
translate!("split-error-unable-to-reopen-file", "file" => filename),
|
||||
)
|
||||
),
|
||||
_ => Error::other(format!(
|
||||
"split: {filename}: {}",
|
||||
uucore::error::strip_errno(&e)
|
||||
)),
|
||||
})?
|
||||
};
|
||||
Ok(BufWriter::new(Box::new(file) as Box<dyn Write>))
|
||||
|
||||
+120
-10
@@ -21,7 +21,9 @@ use std::io::{BufRead, BufReader, BufWriter, ErrorKind, Read, Seek, SeekFrom, Wr
|
||||
use std::path::Path;
|
||||
use thiserror::Error;
|
||||
use uucore::display::Quotable;
|
||||
use uucore::error::{FromIo, UIoError, UResult, USimpleError, UUsageError};
|
||||
use uucore::error::{
|
||||
FromIo, UIoError, UResult, USimpleError, UUsageError, set_exit_code, strip_errno,
|
||||
};
|
||||
use uucore::translate;
|
||||
|
||||
use uucore::parser::parse_size::parse_size_u64;
|
||||
@@ -565,11 +567,54 @@ fn ignorable_io_error(error: &io::Error, settings: &Settings) -> bool {
|
||||
/// If ignorable io error occurs, return number of bytes as if all bytes written
|
||||
/// Should not be used for Kth chunk number sub-strategies
|
||||
/// as those do not work with `--filter` option
|
||||
fn custom_write<T: Write>(bytes: &[u8], writer: &mut T, settings: &Settings) -> io::Result<usize> {
|
||||
///
|
||||
/// When `error_context` is None, acts as simple write wrapper
|
||||
/// When `error_context` is Some((filename, error_reported)), enables error reporting and immediate flushing
|
||||
fn custom_write<T: Write>(
|
||||
bytes: &[u8],
|
||||
writer: &mut T,
|
||||
settings: &Settings,
|
||||
error_context: Option<(&str, &mut bool)>,
|
||||
) -> io::Result<usize> {
|
||||
match writer.write(bytes) {
|
||||
Ok(n) => Ok(n),
|
||||
Ok(n) => {
|
||||
// If error reporting is enabled, flush immediately to catch buffered I/O errors
|
||||
if let Some((filename, error_reported)) = error_context {
|
||||
match writer.flush() {
|
||||
Ok(()) => Ok(n),
|
||||
Err(e) if ignorable_io_error(&e, settings) => Ok(bytes.len()),
|
||||
Err(e) => {
|
||||
if !*error_reported {
|
||||
uucore::show_error!("{}: {}", filename, strip_errno(&e));
|
||||
set_exit_code(1);
|
||||
*error_reported = true;
|
||||
}
|
||||
Err(io::Error::new(
|
||||
ErrorKind::Other,
|
||||
translate!("split-error-flush-already-reported"),
|
||||
))
|
||||
}
|
||||
}
|
||||
} else {
|
||||
Ok(n)
|
||||
}
|
||||
}
|
||||
Err(e) if ignorable_io_error(&e, settings) => Ok(bytes.len()),
|
||||
Err(e) => Err(e),
|
||||
Err(e) => {
|
||||
if let Some((filename, error_reported)) = error_context {
|
||||
if !*error_reported {
|
||||
uucore::show_error!("{}: {}", filename, strip_errno(&e));
|
||||
set_exit_code(1);
|
||||
*error_reported = true;
|
||||
}
|
||||
Err(io::Error::new(
|
||||
ErrorKind::Other,
|
||||
translate!("split-error-write-already-reported"),
|
||||
))
|
||||
} else {
|
||||
Err(e)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -713,6 +758,12 @@ struct ByteChunkWriter<'a> {
|
||||
|
||||
/// Iterator that yields filenames for each chunk.
|
||||
filename_iterator: FilenameIterator<'a>,
|
||||
|
||||
/// Current filename being written to.
|
||||
current_filename: String,
|
||||
|
||||
/// Whether an error has already been reported for the current file.
|
||||
error_reported: bool,
|
||||
}
|
||||
|
||||
impl<'a> ByteChunkWriter<'a> {
|
||||
@@ -732,10 +783,28 @@ impl<'a> ByteChunkWriter<'a> {
|
||||
num_chunks_written: 0,
|
||||
inner,
|
||||
filename_iterator,
|
||||
current_filename: filename,
|
||||
error_reported: false,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ByteChunkWriter<'_> {
|
||||
fn drop(&mut self) {
|
||||
// Ensure final flush to catch any buffered write errors, but only report if not already reported
|
||||
if !self.error_reported {
|
||||
if let Err(e) = self.inner.flush() {
|
||||
uucore::show_error!(
|
||||
"split: {}: final flush failed: {}",
|
||||
self.current_filename,
|
||||
strip_errno(&e)
|
||||
);
|
||||
set_exit_code(1);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Write for ByteChunkWriter<'_> {
|
||||
/// Implements `--bytes=SIZE`
|
||||
fn write(&mut self, mut buf: &[u8]) -> io::Result<usize> {
|
||||
@@ -751,6 +820,16 @@ impl Write for ByteChunkWriter<'_> {
|
||||
}
|
||||
|
||||
if self.num_bytes_remaining_in_current_chunk == 0 {
|
||||
// Flush the current writer before switching to a new file to catch any delayed write errors
|
||||
if let Err(e) = self.inner.flush() {
|
||||
uucore::show_error!("{}: {}", self.current_filename, strip_errno(&e));
|
||||
set_exit_code(1);
|
||||
return Err(io::Error::new(
|
||||
ErrorKind::Other,
|
||||
translate!("split-error-flush-before-file-switch"),
|
||||
));
|
||||
}
|
||||
|
||||
// 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;
|
||||
@@ -763,6 +842,8 @@ impl Write for ByteChunkWriter<'_> {
|
||||
println!("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
|
||||
@@ -771,7 +852,12 @@ 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 = custom_write(
|
||||
buf,
|
||||
&mut self.inner,
|
||||
self.settings,
|
||||
Some((&self.current_filename, &mut self.error_reported)),
|
||||
)?;
|
||||
self.num_bytes_remaining_in_current_chunk -= num_bytes_written as u64;
|
||||
return Ok(carryover_bytes_written + num_bytes_written);
|
||||
}
|
||||
@@ -782,7 +868,12 @@ 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 = custom_write(
|
||||
&buf[..i],
|
||||
&mut self.inner,
|
||||
self.settings,
|
||||
Some((&self.current_filename, &mut self.error_reported)),
|
||||
)?;
|
||||
self.num_bytes_remaining_in_current_chunk -= num_bytes_written as u64;
|
||||
|
||||
// It's possible that the underlying writer did not
|
||||
@@ -799,7 +890,17 @@ impl Write for ByteChunkWriter<'_> {
|
||||
}
|
||||
}
|
||||
fn flush(&mut self) -> io::Result<()> {
|
||||
self.inner.flush()
|
||||
match self.inner.flush() {
|
||||
Ok(()) => Ok(()),
|
||||
Err(e) => {
|
||||
uucore::show_error!("{}: {}", self.current_filename, strip_errno(&e));
|
||||
set_exit_code(1);
|
||||
Err(io::Error::new(
|
||||
ErrorKind::Other,
|
||||
translate!("split-error-flush-in-chunk-writer"),
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -891,7 +992,8 @@ impl Write for LineChunkWriter<'_> {
|
||||
// Write the line, starting from *after* the previous
|
||||
// separator character and ending *after* the current
|
||||
// separator character.
|
||||
let num_bytes_written = custom_write(&buf[prev..=i], &mut self.inner, self.settings)?;
|
||||
let num_bytes_written =
|
||||
custom_write(&buf[prev..=i], &mut self.inner, self.settings, None)?;
|
||||
total_bytes_written += num_bytes_written;
|
||||
prev = i + 1;
|
||||
self.num_lines_remaining_in_current_chunk -= 1;
|
||||
@@ -907,7 +1009,7 @@ impl Write for LineChunkWriter<'_> {
|
||||
self.num_lines_remaining_in_current_chunk = self.chunk_size;
|
||||
}
|
||||
let num_bytes_written =
|
||||
custom_write(&buf[prev..buf.len()], &mut self.inner, self.settings)?;
|
||||
custom_write(&buf[prev..buf.len()], &mut self.inner, self.settings, None)?;
|
||||
total_bytes_written += num_bytes_written;
|
||||
}
|
||||
Ok(total_bytes_written)
|
||||
@@ -1606,7 +1708,15 @@ fn split(settings: &Settings) -> UResult<()> {
|
||||
// allowable filenames, we use `ErrorKind::Other` to
|
||||
// indicate that. A special error message needs to be
|
||||
// printed in that case.
|
||||
ErrorKind::Other => Err(USimpleError::new(1, format!("{e}"))),
|
||||
ErrorKind::Other => {
|
||||
let error_msg = format!("{e}");
|
||||
if error_msg.is_empty() {
|
||||
// This is a handled error, return error to stop processing
|
||||
Err(USimpleError::new(1, ""))
|
||||
} else {
|
||||
Err(USimpleError::new(1, error_msg))
|
||||
}
|
||||
}
|
||||
_ => Err(uio_error!(
|
||||
e,
|
||||
"{}",
|
||||
|
||||
@@ -2091,3 +2091,46 @@ fn test_split_directory_already_exists() {
|
||||
.no_stdout()
|
||||
.stderr_is("split: xaa: Is a directory\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(target_os = "linux")]
|
||||
fn test_split_io_error() {
|
||||
use std::process::Command;
|
||||
|
||||
let (at, mut ucmd) = at_and_ucmd!();
|
||||
|
||||
// Use ln command to create a symlink to /dev/full, like the GNU test does
|
||||
// This is more reliable in various test environments
|
||||
let ln_result = Command::new("ln")
|
||||
.args(&["-sf", "/dev/full", "xaa"])
|
||||
.current_dir(at.as_string())
|
||||
.status();
|
||||
|
||||
if ln_result.is_err() || !ln_result.unwrap().success() {
|
||||
// Skip the test if we can't create the symlink (might happen in some test environments)
|
||||
eprintln!("Skipping I/O error test: cannot create symlink to /dev/full");
|
||||
return;
|
||||
}
|
||||
|
||||
// Create input with 2 bytes to be split into 1-byte chunks
|
||||
at.write("input", "ab");
|
||||
|
||||
// split should fail with exit code 1 when writing to /dev/full
|
||||
ucmd.args(&["-b", "1", "input"])
|
||||
.fails_with_code(1)
|
||||
.no_stdout()
|
||||
.stderr_contains("split: xaa: No space left on device");
|
||||
|
||||
// The symlink should still exist after the failed split
|
||||
// Note: at.file_exists() doesn't handle symlinks properly, so we use Path::exists() as fallback
|
||||
let xaa_path = at.as_string().to_owned() + "/xaa";
|
||||
assert!(
|
||||
at.file_exists("xaa") || Path::new(&xaa_path).exists(),
|
||||
"Expected xaa symlink to exist after failed split"
|
||||
);
|
||||
// But split should not have continued to create additional files
|
||||
assert!(
|
||||
!at.file_exists("xab"),
|
||||
"Expected xab to not exist after split failure"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user