bugfix: Reuse the same file pointer for repeated find output

src/find/matchers/ls.rs

@@ -7,6 +7,7 @@
 use std::{
     fs::File,
     io::{stderr, Write},
+    sync::Arc,
 };
 
 use super::{Matcher, MatcherIO, WalkEntry};
@@ -110,11 +111,11 @@
 }
 
 pub struct Ls {
-    output_file: Option<File>,
+    output_file: Option<Arc<File>>,
 }
 
 impl Ls {
-    pub fn new(output_file: Option<File>) -> Self {
+    pub fn new(output_file: Option<Arc<File>>) -> Self {
         Self { output_file }
     }
 
@@ -268,7 +269,7 @@
 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.as_ref(), true);
         } else {
             self.print(
                 file_info,

src/find/matchers/mod.rs

@@ -31,6 +31,18 @@
 mod type_matcher;
 mod user;
 
+use ::regex::Regex;
+use chrono::{DateTime, Datelike, NaiveDateTime, Utc};
+use fs::FileSystemMatcher;
+use ls::Ls;
+use std::collections::HashMap;
+use std::fs::{File, Metadata};
+use std::path::Path;
+use std::sync::Arc;
+use std::time::SystemTime;
+use std::{error::Error, str::FromStr};
+use uucore::fs::FileInformation;
+
 use self::access::AccessMatcher;
 use self::delete::DeleteMatcher;
 use self::empty::EmptyMatcher;
@@ -58,18 +70,7 @@
 };
 use self::type_matcher::{TypeMatcher, XtypeMatcher};
 use self::user::{NoUserMatcher, UserMatcher};
-use ::regex::Regex;
-use chrono::{DateTime, Datelike, NaiveDateTime, Utc};
-use fs::FileSystemMatcher;
-use ls::Ls;
-use std::{
-    error::Error,
-    fs::{File, Metadata},
-    io::Read,
-    path::Path,
-    str::FromStr,
-    time::SystemTime,
-};
+use std::io::Read;
 
 use super::{Config, Dependencies};
 
@@ -272,13 +273,45 @@
     }
 }
 
+// Used on file output arguments.
+// If yes, use the same file pointer.
+struct FileMemoizer {
+    mem: HashMap<FileInformation, Arc<File>>,
+}
+impl FileMemoizer {
+    fn new() -> Self {
+        Self {
+            mem: HashMap::new(),
+        }
+    }
+    fn get_or_create_file(&mut self, path: &str) -> Result<Arc<File>, Box<dyn Error>> {
+        let mut file_info = FileInformation::from_path(path, true);
+        match file_info {
+            Ok(info) => {
+                let file = self
+                    .mem
+                    .entry(info)
+                    .or_insert(Arc::new(File::create(path)?));
+                Ok(file.clone())
+            }
+            Err(_) => {
+                let file = Arc::new(File::create(path)?);
+                file_info = FileInformation::from_path(path, true);
+                self.mem.insert(file_info?, file.clone());
+                Ok(file)
+            }
+        }
+    }
+}
+
 /// Builds a single `AndMatcher` containing the Matcher objects corresponding
 /// to the passed in predicate arguments.
 pub fn build_top_level_matcher(
     args: &[&str],
     config: &mut Config,
 ) -> Result<Box<dyn Matcher>, Box<dyn Error>> {
-    let (_, top_level_matcher) = (build_matcher_tree(args, config, 0, false))?;
+    let mut file_mem = FileMemoizer::new();
+    let (_, top_level_matcher) = (build_matcher_tree(args, config, &mut file_mem, 0, false))?;
 
     // if the matcher doesn't have any side-effects, then we default to printing
     if !top_level_matcher.has_side_effects() {
@@ -421,20 +454,14 @@
     }
 }
 
-/// Creates a file if it doesn't exist.
-/// If it does exist, it will be overwritten.
-fn get_or_create_file(path: &str) -> Result<File, Box<dyn Error>> {
-    let file = File::create(path)?;
-    Ok(file)
-}
-
 /// The main "translate command-line args into a matcher" function. Will call
 /// itself recursively if it encounters an opening bracket. A successful return
 /// consists of a tuple containing the new index into the args array to use (if
 /// called recursively) and the resulting matcher.
 fn build_matcher_tree(
     args: &[&str],
     config: &mut Config,
+    file_mem: &mut FileMemoizer,
     arg_index: usize,
     mut expecting_bracket: bool,
 ) -> Result<(usize, Box<dyn Matcher>), Box<dyn Error>> {
@@ -465,7 +492,7 @@
                 }
                 i += 1;
 
-                let file = get_or_create_file(args[i])?;
+                let file = file_mem.get_or_create_file(args[i])?;
                 Some(Printer::new(PrintDelimiter::Newline, Some(file)).into_box())
             }
             "-fprintf" => {
@@ -477,7 +504,7 @@
                 // Args + 1: output file path
                 // Args + 2: format string
                 i += 1;
-                let file = get_or_create_file(args[i])?;
+                let file = file_mem.get_or_create_file(args[i])?;
                 i += 1;
                 Some(Printf::new(args[i], Some(file))?.into_box())
             }
@@ -487,7 +514,7 @@
                 }
                 i += 1;
 
-                let file = get_or_create_file(args[i])?;
+                let file = file_mem.get_or_create_file(args[i])?;
                 Some(Printer::new(PrintDelimiter::Null, Some(file)).into_box())
             }
             "-ls" => Some(Ls::new(None).into_box()),
@@ -497,7 +524,7 @@
                 }
                 i += 1;
 
-                let file = get_or_create_file(args[i])?;
+                let file = file_mem.get_or_create_file(args[i])?;
                 Some(Ls::new(Some(file)).into_box())
             }
             "-true" => Some(TrueMatcher.into_box()),
@@ -814,7 +841,8 @@
                 None
             }
             "(" => {
-                let (new_arg_index, sub_matcher) = build_matcher_tree(args, config, i + 1, true)?;
+                let (new_arg_index, sub_matcher) =
+                    build_matcher_tree(args, config, file_mem, i + 1, true)?;
                 i = new_arg_index;
                 Some(sub_matcher)
             }
@@ -1797,30 +1825,4 @@
             .expect("-version should stop parsing");
         assert!(config.version_requested);
     }
-
-    #[test]
-    fn get_or_create_file_test() {
-        use std::fs;
-
-        // 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");
-        assert!(file.is_ok());
-
-        let file = get_or_create_file("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");
-            assert!(result.is_err());
-        }
-
-        let _ = fs::remove_file("test_data/get_or_create_file_test");
-    }
 }

src/find/matchers/printer.rs

@@ -6,6 +6,7 @@
 
 use std::fs::File;
 use std::io::{stderr, Write};
+use std::sync::Arc;
 
 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<File>,
+    output_file: Option<Arc<File>>,
 }
 
 impl Printer {
-    pub fn new(delimiter: PrintDelimiter, output_file: Option<File>) -> Self {
+    pub fn new(delimiter: PrintDelimiter, output_file: Option<Arc<File>>) -> Self {
         Self {
             delimiter,
             output_file,
@@ -71,7 +72,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.as_ref(), true);
         } else {
             self.print(
                 file_info,
@@ -127,7 +128,7 @@ mod tests {
         let dev_full = File::open("/dev/full").unwrap();
         let abbbc = get_dir_entry_for("./test_data/simple", "abbbc");
 
-        let matcher = Printer::new(PrintDelimiter::Newline, Some(dev_full));
+        let matcher = Printer::new(PrintDelimiter::Newline, Some(Arc::new(dev_full)));
         let deps = FakeDependencies::new();
 
         assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io()));

src/find/matchers/printf.rs

@@ -7,6 +7,7 @@
 use std::error::Error;
 use std::fs::{self, File};
 use std::path::Path;
+use std::sync::Arc;
 use std::time::SystemTime;
 use std::{borrow::Cow, io::Write};
 
@@ -572,11 +573,11 @@ fn format_directive<'entry>(
 /// find's printf syntax.
 pub struct Printf {
     format: FormatString,
-    output_file: Option<File>,
+    output_file: Option<Arc<File>>,
 }
 
 impl Printf {
-    pub fn new(format: &str, output_file: Option<File>) -> Result<Self, Box<dyn Error>> {
+    pub fn new(format: &str, output_file: Option<Arc<File>>) -> Result<Self, Box<dyn Error>> {
         Ok(Self {
             format: FormatString::parse(format)?,
             output_file,
@@ -624,7 +625,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.as_ref());
         } else {
             self.print(file_info, &mut *matcher_io.deps.get_output().borrow_mut());
         }