comm: Properly handle I/O errors when reading input

src/uu/comm/src/comm.rs

@@ -194,7 +194,14 @@ fn write_line_with_delimiter<W: Write>(writer: &mut W, delim: &[u8], line: &[u8]
     Ok(())
 }
 
-fn comm(a: &mut LineReader, b: &mut LineReader, delim: &str, opts: &ArgMatches) -> UResult<()> {
+fn comm(
+    a: &mut LineReader,
+    b: &mut LineReader,
+    filename1: &OsString,
+    filename2: &OsString,
+    delim: &str,
+    opts: &ArgMatches,
+) -> UResult<()> {
     let width_col_1 = usize::from(!opts.get_flag(options::COLUMN_1));
     let width_col_2 = usize::from(!opts.get_flag(options::COLUMN_2));
 
@@ -204,9 +211,13 @@ fn comm(a: &mut LineReader, b: &mut LineReader, delim: &str, opts: &ArgMatches)
     let mut writer = BufWriter::new(io::stdout().lock());
 
     let ra = &mut Vec::new();
-    let mut na = a.read_line(ra);
+    let mut na = a
+        .read_line(ra)
+        .map_err_context(|| filename1.maybe_quote().to_string())?;
     let rb = &mut Vec::new();
-    let mut nb = b.read_line(rb);
+    let mut nb = b
+        .read_line(rb)
+        .map_err_context(|| filename2.maybe_quote().to_string())?;
 
     let mut total_col_1 = 0;
     let mut total_col_2 = 0;
@@ -218,31 +229,19 @@ fn comm(a: &mut LineReader, b: &mut LineReader, delim: &str, opts: &ArgMatches)
     // Determine if we should perform order checking
     let should_check_order = !no_check_order
         && (check_order
-            || if let (Some(file1), Some(file2)) = (
-                opts.get_one::<OsString>(options::FILE_1),
-                opts.get_one::<OsString>(options::FILE_2),
-            ) {
-                !(paths_refer_to_same_file(file1.as_os_str(), file2.as_os_str(), true)
-                    || are_files_identical(Path::new(file1), Path::new(file2)).unwrap_or(false))
-            } else {
-                true
-            });
+            || !(paths_refer_to_same_file(filename1.as_os_str(), filename2.as_os_str(), true)
+                || are_files_identical(Path::new(filename1), Path::new(filename2))
+                    .unwrap_or(false)));
 
     let mut checker1 = OrderChecker::new(FileNumber::One, check_order);
     let mut checker2 = OrderChecker::new(FileNumber::Two, check_order);
     let mut input_error = false;
 
-    while na.is_ok() || nb.is_ok() {
-        let ord = match (na.is_ok(), nb.is_ok()) {
-            (false, true) => Ordering::Greater,
-            (true, false) => Ordering::Less,
-            (true, true) => match (&na, &nb) {
-                (&Ok(0), &Ok(0)) => break,
-                (&Ok(0), _) => Ordering::Greater,
-                (_, &Ok(0)) => Ordering::Less,
-                _ => ra.cmp(&rb),
-            },
-            _ => unreachable!(),
+    while na != 0 || nb != 0 {
+        let ord = match (na, nb) {
+            (0, _) => Ordering::Greater,
+            (_, 0) => Ordering::Less,
+            (_, _) => ra.as_slice().cmp(rb.as_slice()),
         };
 
         match ord {
@@ -256,7 +255,9 @@ fn comm(a: &mut LineReader, b: &mut LineReader, delim: &str, opts: &ArgMatches)
                         .map_err_context(|| translate!("comm-error-write"))?;
                 }
                 ra.clear();
-                na = a.read_line(ra);
+                na = a
+                    .read_line(ra)
+                    .map_err_context(|| filename1.maybe_quote().to_string())?;
                 total_col_1 += 1;
             }
             Ordering::Greater => {
@@ -267,7 +268,9 @@ fn comm(a: &mut LineReader, b: &mut LineReader, delim: &str, opts: &ArgMatches)
                     write_line_with_delimiter(&mut writer, delim_col_2.as_bytes(), rb)?;
                 }
                 rb.clear();
-                nb = b.read_line(rb);
+                nb = b
+                    .read_line(rb)
+                    .map_err_context(|| filename2.maybe_quote().to_string())?;
                 total_col_2 += 1;
             }
             Ordering::Equal => {
@@ -280,8 +283,12 @@ fn comm(a: &mut LineReader, b: &mut LineReader, delim: &str, opts: &ArgMatches)
                 }
                 ra.clear();
                 rb.clear();
-                na = a.read_line(ra);
-                nb = b.read_line(rb);
+                na = a
+                    .read_line(ra)
+                    .map_err_context(|| filename1.maybe_quote().to_string())?;
+                nb = b
+                    .read_line(rb)
+                    .map_err_context(|| filename2.maybe_quote().to_string())?;
                 total_col_3 += 1;
             }
         }
@@ -360,7 +367,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         delim => delim,
     };
 
-    comm(&mut f1, &mut f2, delim, &matches)
+    comm(&mut f1, &mut f2, filename1, filename2, delim, &matches)
 }
 
 pub fn uu_app() -> Command {

tests/by-util/test_comm.rs

@@ -711,3 +711,19 @@ fn test_comm_anonymous_pipes() {
         .succeeds()
         .stdout_is("99999\n");
 }
+
+#[test]
+#[cfg(all(target_os = "linux", not(target_env = "musl")))]
+fn test_read_error() {
+    new_ucmd!()
+        .arg("/proc/self/mem")
+        .arg("/dev/null")
+        .fails()
+        .stderr_contains("comm: /proc/self/mem: Input/output error");
+
+    new_ucmd!()
+        .arg("/dev/null")
+        .arg("/proc/self/mem")
+        .fails()
+        .stderr_contains("comm: /proc/self/mem: Input/output error");
+}