Skip to content

Commit 75f45e8

Browse files
comm: fix comparison when reading from pipes (#9545)
* comm: fix comparison when reading from pipes Use case is that two files are piped into comm, i.e. in bash comm <(cat file1) <(cat file2) Before the fix, comm reads from the pipes twice. Once in "fn comm" and once in "fn are_files_identical". As such, part of the data is skipped in comparison which leads to wrong output. This is fixed by skipping the file comparison in case one of the files is not a regular file. * comm: add test for reading from pipes
1 parent 7707b72 commit 75f45e8

3 files changed

Lines changed: 45 additions & 0 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ uutests.workspace = true
549549
uucore = { workspace = true, features = [
550550
"mode",
551551
"entries",
552+
"pipes",
552553
"process",
553554
"signals",
554555
"utmpx",

src/uu/comm/src/comm.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,11 @@ pub fn are_files_identical(path1: &Path, path2: &Path) -> io::Result<bool> {
136136
return Ok(false);
137137
}
138138

139+
// only proceed if both are regular files
140+
if !metadata1.is_file() || !metadata2.is_file() {
141+
return Ok(false);
142+
}
143+
139144
let file1 = File::open(path1)?;
140145
let file2 = File::open(path2)?;
141146

tests/by-util/test_comm.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,3 +648,42 @@ fn test_comm_eintr_handling() {
648648
.stdout_contains("line2")
649649
.stdout_contains("line3");
650650
}
651+
652+
#[test]
653+
#[cfg(any(target_os = "linux", target_os = "android"))]
654+
fn test_comm_anonymous_pipes() {
655+
use std::{io::Write, os::fd::AsRawFd, process};
656+
use uucore::pipes::pipe;
657+
658+
let scene = TestScenario::new(util_name!());
659+
660+
// Open two anonymous pipes
661+
let (comm1_reader, mut comm1_writer) = pipe().unwrap();
662+
let (comm2_reader, mut comm2_writer) = pipe().unwrap();
663+
664+
// comm reads the data in chunks
665+
// make content large enough, so that at least two chunks are read
666+
// default buffer size is 8192, so with 6 characters (5 digits + \n) per line we need to write at least 1366 lines
667+
668+
// write 1500 lines into comm1: 00000\n00001\n...01500\n
669+
let mut content = String::new();
670+
for i in 0..1500 {
671+
content.push_str(&format!("{i:05}\n"));
672+
}
673+
assert!(comm1_writer.write_all(content.as_bytes()).is_ok());
674+
drop(comm1_writer);
675+
676+
// write into comm2: 00000\n00001\n...01500\n99999\n
677+
content.push_str("99999\n");
678+
assert!(comm2_writer.write_all(content.as_bytes()).is_ok());
679+
drop(comm2_writer);
680+
681+
// run comm, showing unique lines in second input
682+
let comm1_fd = format!("/proc/{}/fd/{}", process::id(), comm1_reader.as_raw_fd());
683+
let comm2_fd = format!("/proc/{}/fd/{}", process::id(), comm2_reader.as_raw_fd());
684+
scene
685+
.ucmd()
686+
.args(&["-13", &comm1_fd, &comm2_fd])
687+
.succeeds()
688+
.stdout_is("99999\n");
689+
}

0 commit comments

Comments
 (0)