fix: improve error handling for file operations in tac and ln commands

Author: abendrothj

src/uu/ln/src/ln.rs

@@ -430,24 +430,20 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
     if settings.symbolic {
         symlink(&source, dst)?;
     } else {
-        // Cannot create hard link to a directory directly
-        // We can however create hard link to a symlink that points to a directory, so long as -L is not passed
-        if src.is_dir() && (!src.is_symlink() || settings.logical) {
-            return Err(LnError::FailedToCreateHardLinkDir(source.to_path_buf()).into());
-        }
-
         let p = if settings.logical && source.is_symlink() {
-            // if we want to have an hard link,
-            // source is a symlink and -L is passed
-            // we want to resolve the symlink to create the hardlink
             fs::canonicalize(&source)
                 .map_err_context(|| translate!("ln-failed-to-access", "file" => source.quote()))?
         } else {
             source.to_path_buf()
         };
-        fs::hard_link(p, dst).map_err_context(|| {
-            translate!("ln-failed-to-create-hard-link", "source" => source.quote(), "dest" => dst.quote())
-        })?;
+        if let Err(e) = fs::hard_link(&p, dst) {
+            if p.is_dir() {
+                return Err(LnError::FailedToCreateHardLinkDir(source.to_path_buf()).into());
+            }
+            return Err(e).map_err_context(|| {
+                translate!("ln-failed-to-create-hard-link", "source" => source.quote(), "dest" => dst.quote())
+            });
+        }
     }
 
     if settings.verbose {

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

@@ -6,7 +6,6 @@ tac-help-separator = use STRING as the separator instead of newline
 
 # Error messages
 tac-error-invalid-regex = invalid regular expression: { $error }
-tac-error-invalid-directory-argument = { $argument }: read error: Is a directory
-tac-error-file-not-found = failed to open { $filename } for reading: No such file or directory
-tac-error-read-error = failed to read from { $filename }: { $error }
+tac-error-open-error = failed to open { $filename } for reading: { $error }
+tac-error-read-error = { $filename }: read error: { $error }
 tac-error-write-error = failed to write to stdout: { $error }

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

@@ -6,7 +6,6 @@ tac-help-separator = utiliser CHAÎNE comme séparateur au lieu du saut de ligne
 
 # Messages d'erreur
 tac-error-invalid-regex = expression régulière invalide : { $error }
-tac-error-file-not-found = échec de l'ouverture de { $filename } en lecture : Aucun fichier ou répertoire de ce type
-tac-error-read-error = échec de la lecture depuis { $filename } : { $error }
+tac-error-open-error = échec de l'ouverture de { $filename } en lecture : { $error }
+tac-error-read-error = { $filename } : erreur de lecture : { $error }
 tac-error-write-error = échec de l'écriture vers stdout : { $error }
-tac-error-invalid-directory-argument = { $argument } : erreur de lecture : Est un répertoire

src/uu/tac/src/error.rs

@@ -7,31 +7,31 @@
 use std::ffi::OsString;
 use thiserror::Error;
 use uucore::display::Quotable;
-use uucore::error::UError;
+use uucore::error::{UError, strip_errno};
 use uucore::translate;
 
 #[derive(Debug, Error)]
 pub enum TacError {
     /// A regular expression given by the user is invalid.
     #[error("{}", translate!("tac-error-invalid-regex", "error" => .0))]
     InvalidRegex(regex::Error),
-    /// The argument to tac is a directory.
-    #[error("{}", translate!("tac-error-invalid-directory-argument", "argument" => .0.maybe_quote()))]
-    InvalidDirectoryArgument(OsString),
-    /// The specified file is not found on the filesystem.
-    #[error("{}", translate!("tac-error-file-not-found", "filename" => .0.quote()))]
-    FileNotFound(OsString),
+    /// An error opening a file for reading.
+    ///
+    /// The parameters are the name of the file and the underlying
+    /// [`std::io::Error`] that caused this error.
+    #[error("{}", translate!("tac-error-open-error", "filename" => .0.quote(), "error" => strip_errno(.1)))]
+    OpenError(OsString, std::io::Error),
     /// An error reading the contents of a file or stdin.
     ///
     /// The parameters are the name of the file and the underlying
     /// [`std::io::Error`] that caused this error.
-    #[error("{}", translate!("tac-error-read-error", "filename" => .0.quote(), "error" => .1))]
+    #[error("{}", translate!("tac-error-read-error", "filename" => .0.maybe_quote(), "error" => strip_errno(.1)))]
     ReadError(OsString, std::io::Error),
     /// An error writing the (reversed) contents of a file or stdin.
     ///
     /// The parameter is the underlying [`std::io::Error`] that caused
     /// this error.
-    #[error("{}", translate!("tac-error-write-error", "error" => .0))]
+    #[error("{}", translate!("tac-error-write-error", "error" => strip_errno(.0)))]
     WriteError(std::io::Error),
 }
 

src/uu/tac/src/tac.rs

@@ -13,7 +13,7 @@ use memmap2::Mmap;
 use std::ffi::{OsStr, OsString};
 use std::io::{BufWriter, Read, Write, stdin, stdout};
 use std::{
-    fs::{File, read},
+    fs::File,
     io::copy,
     path::Path,
 };
@@ -378,31 +378,26 @@ fn tac(filenames: &[OsString], before: bool, regex: bool, separator: &OsStr) ->
             }
         } else {
             let path = Path::new(filename);
-            if path.is_dir() {
-                let e: Box<dyn UError> =
-                    TacError::InvalidDirectoryArgument(filename.clone()).into();
-                show!(e);
-                continue;
-            }
-
-            if path.metadata().is_err() {
-                let e: Box<dyn UError> = TacError::FileNotFound(filename.clone()).into();
-                show!(e);
-                continue;
-            }
+            let mut file = match File::open(path) {
+                Ok(f) => f,
+                Err(e) => {
+                    show!(TacError::OpenError(filename.clone(), e));
+                    continue;
+                }
+            };
 
-            if let Some(mmap1) = try_mmap_path(path) {
+            if let Some(mmap1) = try_mmap_file(&file) {
                 mmap = mmap1;
                 &mmap
             } else {
-                match read(path) {
-                    Ok(buf1) => {
-                        buf = buf1;
+                let mut contents = Vec::new();
+                match file.read_to_end(&mut contents) {
+                    Ok(_) => {
+                        buf = contents;
                         &buf
                     }
                     Err(e) => {
-                        let e: Box<dyn UError> = TacError::ReadError(filename.clone(), e).into();
-                        show!(e);
+                        show!(TacError::ReadError(filename.clone(), e));
                         continue;
                     }
                 }
@@ -458,14 +453,10 @@ fn buffer_stdin() -> std::io::Result<StdinData> {
     }
 }
 
-fn try_mmap_path(path: &Path) -> Option<Mmap> {
-    let file = File::open(path).ok()?;
-
+fn try_mmap_file(file: &File) -> Option<Mmap> {
     // SAFETY: If the file is truncated while we map it, SIGBUS will be raised
     // and our process will be terminated, thus preventing access of invalid memory.
-    let mmap = unsafe { Mmap::map(&file).ok()? };
-
-    Some(mmap)
+    unsafe { Mmap::map(file).ok() }
 }
 
 #[cfg(test)]