split: fix I/O error handling for device full conditions

Should fix tests/split/split-io-err.sh

Author: sylvestre

src/uu/split/locales/en-US.ftl

@@ -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.

src/uu/split/locales/fr-FR.ftl

@@ -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

src/uu/split/src/platform/unix.rs

@@ -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>))

src/uu/split/src/split.rs

@@ -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,48 @@ 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::other(""))
+                    }
+                }
+            } 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::other(""))
+            } else {
+                Err(e)
+            }
+        }
     }
 }
 
@@ -713,6 +752,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 +777,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 +814,13 @@ 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::other(""));
+                }
+
                 // 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 +833,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 +843,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 +859,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 +881,14 @@ 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::other(""))
+            }
+        }
     }
 }
 
@@ -891,7 +980,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 +997,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 +1696,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,
                         "{}",

tests/by-util/test_split.rs

@@ -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().clone() + "/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"
+    );
+}