diff --git a/src/find/matchers/ls.rs b/src/find/matchers/ls.rs index 9b05f82d..771c9880 100644 --- a/src/find/matchers/ls.rs +++ b/src/find/matchers/ls.rs @@ -7,6 +7,7 @@ use chrono::DateTime; use std::{ fs::File, io::{stderr, Write}, + rc::Rc, }; use super::{Matcher, MatcherIO, WalkEntry}; @@ -110,11 +111,11 @@ fn format_permissions(file_attributes: u32) -> String { } pub struct Ls { - output_file: Option, + output_file: Option>, } impl Ls { - pub fn new(output_file: Option) -> Self { + pub fn new(output_file: Option>) -> Self { Self { output_file } } @@ -270,7 +271,7 @@ impl Ls { impl Matcher for Ls { fn matches(&self, file_info: &WalkEntry, matcher_io: &mut MatcherIO) -> bool { if let Some(file) = &self.output_file { - self.print(file_info, matcher_io, file, true); + self.print(file_info, matcher_io, &**file, true); } else { self.print( file_info, diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index e35ebbbb..e2a1361a 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -67,6 +67,7 @@ use std::{ fs::{File, Metadata}, io::Read, path::Path, + rc::Rc, str::FromStr, time::SystemTime, }; @@ -437,10 +438,21 @@ fn parse_str_to_newer_args(input: &str) -> Option<(String, String)> { } } -/// Creates a file if it doesn't exist. -/// If it does exist, it will be overwritten. -fn get_or_create_file(path: &str) -> Result> { - let file = File::create(path)?; +/// Returns the output file for `path`, creating (and truncating) it the first +/// time it is requested. +/// +/// Later requests for the same path reuse the handle opened earlier, so that +/// several output predicates writing to one file share a single file offset +/// instead of overwriting each other. +fn get_or_create_file(config: &mut Config, path: &str) -> Result, Box> { + if let Some(file) = config.output_files.get(path) { + return Ok(Rc::clone(file)); + } + + let file = Rc::new(File::create(path)?); + config + .output_files + .insert(path.to_string(), Rc::clone(&file)); Ok(file) } @@ -481,7 +493,7 @@ fn build_matcher_tree( } i += 1; - let file = get_or_create_file(args[i])?; + let file = get_or_create_file(config, args[i])?; Some(Printer::new(PrintDelimiter::Newline, Some(file)).into_box()) } "-fprintf" => { @@ -493,7 +505,7 @@ fn build_matcher_tree( // Args + 1: output file path // Args + 2: format string i += 1; - let file = get_or_create_file(args[i])?; + let file = get_or_create_file(config, args[i])?; i += 1; Some(Printf::new(args[i], Some(file))?.into_box()) } @@ -503,7 +515,7 @@ fn build_matcher_tree( } i += 1; - let file = get_or_create_file(args[i])?; + let file = get_or_create_file(config, args[i])?; Some(Printer::new(PrintDelimiter::Null, Some(file)).into_box()) } "-ls" => Some(Ls::new(None).into_box()), @@ -513,7 +525,7 @@ fn build_matcher_tree( } i += 1; - let file = get_or_create_file(args[i])?; + let file = get_or_create_file(config, args[i])?; Some(Ls::new(Some(file)).into_box()) } "-true" => Some(TrueMatcher.into_box()), @@ -1059,6 +1071,8 @@ mod tests { use super::*; use crate::find::tests::fix_up_slashes; use crate::find::tests::FakeDependencies; + use std::io::Write; + use tempfile::Builder; /// Helper function for tests to get a [WalkEntry] object. root should /// probably be a string starting with `test_data/` (cargo's tests run with @@ -1938,25 +1952,66 @@ mod tests { fn get_or_create_file_test() { use std::fs; + let mut config = Config::default(); + // remove file if hard link file exist. // But you can't delete a file that doesn't exist, // so ignore the error returned here. let _ = fs::remove_file("test_data/get_or_create_file_test"); // test create file - let file = get_or_create_file("test_data/get_or_create_file_test"); + let file = get_or_create_file(&mut config, "test_data/get_or_create_file_test"); assert!(file.is_ok()); - let file = get_or_create_file("test_data/get_or_create_file_test"); + let file = get_or_create_file(&mut config, "test_data/get_or_create_file_test"); assert!(file.is_ok()); // test error when file no permission #[cfg(unix)] { - let result = get_or_create_file("/etc/shadow"); + let result = get_or_create_file(&mut config, "/etc/shadow"); assert!(result.is_err()); } let _ = fs::remove_file("test_data/get_or_create_file_test"); } + + #[test] + fn get_or_create_file_reuses_handle_for_same_path() { + use std::fs; + + let temp_dir = Builder::new().prefix("example").tempdir().unwrap(); + let path = temp_dir.path().join("out"); + let path = path.to_string_lossy().to_string(); + let mut config = Config::default(); + + let first = get_or_create_file(&mut config, &path).unwrap(); + let second = get_or_create_file(&mut config, &path).unwrap(); + assert!(Rc::ptr_eq(&first, &second)); + + // Writes through both handles share one file offset, so neither + // overwrites the other. + writeln!(&*first, "one").unwrap(); + writeln!(&*second, "two").unwrap(); + assert_eq!("one\ntwo\n", fs::read_to_string(&path).unwrap()); + } + + #[test] + fn two_fprints_to_the_same_file_do_not_overwrite_each_other() { + use std::fs; + + let temp_dir = Builder::new().prefix("example").tempdir().unwrap(); + let path = temp_dir.path().join("out"); + let path = path.to_string_lossy().to_string(); + let mut config = Config::default(); + + let matcher = + build_top_level_matcher(&["-fprint", &path, "-fprint", &path], &mut config).unwrap(); + let deps = FakeDependencies::new(); + let abbbc = get_dir_entry_for("test_data/simple", "abbbc"); + matcher.matches(&abbbc, &mut deps.new_matcher_io()); + + let expected = format!("{0}\n{0}\n", abbbc.path().to_string_lossy()); + assert_eq!(expected, fs::read_to_string(&path).unwrap()); + } } diff --git a/src/find/matchers/printer.rs b/src/find/matchers/printer.rs index c26287a1..06ec492b 100644 --- a/src/find/matchers/printer.rs +++ b/src/find/matchers/printer.rs @@ -6,6 +6,7 @@ use std::fs::File; use std::io::{stderr, Write}; +use std::rc::Rc; use super::{Matcher, MatcherIO, WalkEntry}; @@ -26,11 +27,11 @@ impl std::fmt::Display for PrintDelimiter { /// This matcher just prints the name of the file to stdout. pub struct Printer { delimiter: PrintDelimiter, - output_file: Option, + output_file: Option>, } impl Printer { - pub fn new(delimiter: PrintDelimiter, output_file: Option) -> Self { + pub fn new(delimiter: PrintDelimiter, output_file: Option>) -> Self { Self { delimiter, output_file, @@ -72,7 +73,7 @@ impl Printer { impl Matcher for Printer { fn matches(&self, file_info: &WalkEntry, matcher_io: &mut MatcherIO) -> bool { if let Some(file) = &self.output_file { - self.print(file_info, matcher_io, file, true); + self.print(file_info, matcher_io, &**file, true); } else { self.print( file_info, diff --git a/src/find/matchers/printf.rs b/src/find/matchers/printf.rs index 31151db2..b223c83e 100644 --- a/src/find/matchers/printf.rs +++ b/src/find/matchers/printf.rs @@ -7,6 +7,7 @@ use std::error::Error; use std::fs::{self, File}; use std::path::Path; +use std::rc::Rc; use std::time::SystemTime; use std::{borrow::Cow, io::Write}; @@ -586,11 +587,11 @@ fn format_directive<'entry>( /// find's printf syntax. pub struct Printf { format: FormatString, - output_file: Option, + output_file: Option>, } impl Printf { - pub fn new(format: &str, output_file: Option) -> Result> { + pub fn new(format: &str, output_file: Option>) -> Result> { Ok(Self { format: FormatString::parse(format)?, output_file, @@ -638,7 +639,7 @@ impl Printf { impl Matcher for Printf { fn matches(&self, file_info: &WalkEntry, matcher_io: &mut MatcherIO) -> bool { if let Some(file) = &self.output_file { - self.print(file_info, file); + self.print(file_info, &**file); } else { self.print(file_info, &mut *matcher_io.deps.get_output().borrow_mut()); } diff --git a/src/find/mod.rs b/src/find/mod.rs index f6b80751..a611252e 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -8,7 +8,9 @@ pub mod matchers; use matchers::{Follow, WalkEntry}; use std::cell::RefCell; +use std::collections::HashMap; use std::error::Error; +use std::fs::File; #[cfg(unix)] use std::io::IsTerminal; use std::io::{self, stderr, stdout, BufRead, BufReader, Write}; @@ -30,6 +32,9 @@ pub struct Config { follow: Follow, new_paths: Option>, files0_argument: Option, + /// Files opened by output predicates (-fprint, -fprintf, -fprint0, -fls), + /// keyed by the path given on the command line. + output_files: HashMap>, } impl Default for Config { @@ -50,6 +55,7 @@ impl Default for Config { follow: Follow::Never, new_paths: None, // This option exclusively for -files0-from argument. files0_argument: None, //This option also is used for file0-from + output_files: HashMap::new(), } } }