From 0edc9b01b9a801ef60f86ad97dc31deeb08f5c0e Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 9 Aug 2021 16:03:23 +0200 Subject: [PATCH] factor: prevent writing incomplete lines This makes it possible to execute multiple `factor` instances that write to the same output in parallel, without having them interfere. --- src/uu/factor/src/cli.rs | 25 ++++++++++++++-------- tests/by-util/test_factor.rs | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/uu/factor/src/cli.rs b/src/uu/factor/src/cli.rs index 0f5d21362..7963f162f 100644 --- a/src/uu/factor/src/cli.rs +++ b/src/uu/factor/src/cli.rs @@ -10,6 +10,7 @@ extern crate uucore; use std::error::Error; +use std::fmt::Write as FmtWrite; use std::io::{self, stdin, stdout, BufRead, Write}; mod factor; @@ -28,21 +29,29 @@ mod options { pub static NUMBER: &str = "NUMBER"; } -fn print_factors_str(num_str: &str, w: &mut impl io::Write) -> Result<(), Box> { - num_str - .parse::() - .map_err(|e| e.into()) - .and_then(|x| writeln!(w, "{}:{}", x, factor(x)).map_err(|e| e.into())) +fn print_factors_str( + num_str: &str, + w: &mut io::BufWriter, + factors_buffer: &mut String, +) -> Result<(), Box> { + num_str.parse::().map_err(|e| e.into()).and_then(|x| { + factors_buffer.clear(); + writeln!(factors_buffer, "{}:{}", x, factor(x))?; + w.write_all(factors_buffer.as_bytes())?; + Ok(()) + }) } pub fn uumain(args: impl uucore::Args) -> i32 { let matches = uu_app().get_matches_from(args); let stdout = stdout(); - let mut w = io::BufWriter::new(stdout.lock()); + // We use a smaller buffer here to pass a gnu test. 4KiB appears to be the default pipe size for bash. + let mut w = io::BufWriter::with_capacity(4 * 1024, stdout.lock()); + let mut factors_buffer = String::new(); if let Some(values) = matches.values_of(options::NUMBER) { for number in values { - if let Err(e) = print_factors_str(number, &mut w) { + if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) { show_warning!("{}: {}", number, e); } } @@ -51,7 +60,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for line in stdin.lock().lines() { for number in line.unwrap().split_whitespace() { - if let Err(e) = print_factors_str(number, &mut w) { + if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) { show_warning!("{}: {}", number, e); } } diff --git a/tests/by-util/test_factor.rs b/tests/by-util/test_factor.rs index a3f06ea8a..bcb1238bf 100644 --- a/tests/by-util/test_factor.rs +++ b/tests/by-util/test_factor.rs @@ -8,7 +8,10 @@ // spell-checker:ignore (methods) hexdigest +use tempfile::TempDir; + use crate::common::util::*; +use std::fs::OpenOptions; use std::time::SystemTime; #[path = "../../src/uu/factor/sieve.rs"] @@ -24,6 +27,43 @@ use self::sieve::Sieve; const NUM_PRIMES: usize = 10000; const NUM_TESTS: usize = 100; +#[test] +fn test_parallel() { + // factor should only flush the buffer at line breaks + let n_integers = 100_000; + let mut input_string = String::new(); + for i in 0..=n_integers { + input_string.push_str(&(format!("{} ", i))[..]); + } + + let tmp_dir = TempDir::new().unwrap(); + let tmp_dir = AtPath::new(tmp_dir.path()); + tmp_dir.touch("output"); + let output = OpenOptions::new() + .append(true) + .open(tmp_dir.plus("output")) + .unwrap(); + + for mut child in (0..10) + .map(|_| { + new_ucmd!() + .set_stdout(output.try_clone().unwrap()) + .pipe_in(input_string.clone()) + .run_no_wait() + }) + .collect::>() + { + assert_eq!(child.wait().unwrap().code().unwrap(), 0); + } + + let result = TestScenario::new(util_name!()) + .ccmd("sort") + .arg(tmp_dir.plus("output")) + .succeeds(); + let hash_check = sha1::Sha1::from(result.stdout()).hexdigest(); + assert_eq!(hash_check, "cc743607c0ff300ff575d92f4ff0c87d5660c393"); +} + #[test] fn test_first_100000_integers() { extern crate sha1;