From 089f194fd6609668b64a8f9e026d10998d32c8e6 Mon Sep 17 00:00:00 2001
From: Chris Hennick <hennickc@amazon.com>
Date: Sat, 13 May 2023 15:49:54 -0700
Subject: [PATCH] Simplify FileOperation by splitting into BasicFileOperation
 and bool reopen

---
 fuzz/fuzz_targets/fuzz_write.rs | 50 +++++++++++++--------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/fuzz/fuzz_targets/fuzz_write.rs b/fuzz/fuzz_targets/fuzz_write.rs
index 261c7601..7748f5a7 100644
--- a/fuzz/fuzz_targets/fuzz_write.rs
+++ b/fuzz/fuzz_targets/fuzz_write.rs
@@ -13,63 +13,53 @@ pub struct File {
 }
 
 #[derive(Arbitrary,Debug)]
-pub enum FileOperation {
+pub enum BasicFileOperation {
     WriteNormalFile {
         file: File,
         options: zip_next::write::FileOptions,
-        reopen: bool,
     },
     WriteDirectory {
         name: String,
         options: zip_next::write::FileOptions,
-        reopen: bool,
     },
     WriteSymlink {
         name: String,
         target: Box<PathBuf>,
         options: zip_next::write::FileOptions,
-        reopen: bool,
     },
     ShallowCopy {
         base: Box<FileOperation>,
         new_name: String,
-        reopen: bool,
     },
     DeepCopy {
         base: Box<FileOperation>,
         new_name: String,
-        reopen: bool,
     }
 }
 
+#[derive(Arbitrary,Debug)]
+pub struct FileOperation {
+    basic: BasicFileOperation,
+    reopen: bool,
+}
+
 impl FileOperation {
     pub fn get_name(&self) -> String {
-        match self {
-            FileOperation::WriteNormalFile {file, ..} => &file.name,
-            FileOperation::WriteDirectory {name, ..} => name,
-            FileOperation::WriteSymlink {name, ..} => name,
-            FileOperation::ShallowCopy {new_name, ..} => new_name,
-            FileOperation::DeepCopy {new_name, ..} => new_name
+        match &self.basic {
+            BasicFileOperation::WriteNormalFile {file, ..} => &file.name,
+            BasicFileOperation::WriteDirectory {name, ..} => name,
+            BasicFileOperation::WriteSymlink {name, ..} => name,
+            BasicFileOperation::ShallowCopy {new_name, ..} => new_name,
+            BasicFileOperation::DeepCopy {new_name, ..} => new_name
         }.to_owned()
     }
-
-    pub fn should_reopen(&self) -> bool {
-        match self {
-            FileOperation::WriteNormalFile {reopen, ..} => *reopen,
-            FileOperation::ShallowCopy {reopen, ..} => *reopen,
-            FileOperation::DeepCopy {reopen, ..} => *reopen,
-            FileOperation::WriteDirectory {reopen, ..} => *reopen,
-            FileOperation::WriteSymlink {reopen, ..} => *reopen,
-        }
-    }
 }
 
 fn do_operation<T>(writer: &mut RefCell<zip_next::ZipWriter<T>>,
                    operation: FileOperation) -> Result<(), Box<dyn std::error::Error>>
                    where T: Read + Write + Seek {
-    let should_reopen = operation.should_reopen();
-    match operation {
-        FileOperation::WriteNormalFile {file, mut options, ..} => {
+    match operation.basic {
+        BasicFileOperation::WriteNormalFile {file, mut options, ..} => {
             if file.contents.iter().map(Vec::len).sum::<usize>() >= u32::MAX as usize {
                 options = options.large_file(true);
             }
@@ -78,24 +68,24 @@ fn do_operation<T>(writer: &mut RefCell<zip_next::ZipWriter<T>>,
                 writer.borrow_mut().write_all(chunk.as_slice())?;
             }
         }
-        FileOperation::WriteDirectory {name, options, ..} => {
+        BasicFileOperation::WriteDirectory {name, options, ..} => {
             writer.borrow_mut().add_directory(name, options)?;
         }
-        FileOperation::WriteSymlink {name, target, options, ..} => {
+        BasicFileOperation::WriteSymlink {name, target, options, ..} => {
             writer.borrow_mut().add_symlink(name, target.to_string_lossy(), options)?;
         }
-        FileOperation::ShallowCopy {base, ref new_name, .. } => {
+        BasicFileOperation::ShallowCopy {base, ref new_name, .. } => {
             let base_name = base.get_name();
             do_operation(writer, *base)?;
             writer.borrow_mut().shallow_copy_file(&base_name, new_name)?;
         }
-        FileOperation::DeepCopy {base, ref new_name, .. } => {
+        BasicFileOperation::DeepCopy {base, ref new_name, .. } => {
             let base_name = base.get_name();
             do_operation(writer, *base)?;
             writer.borrow_mut().deep_copy_file(&base_name, new_name)?;
         }
     }
-    if should_reopen {
+    if operation.reopen {
         let new_writer = zip_next::ZipWriter::new_append(writer.borrow_mut().finish().unwrap()).unwrap();
         *writer = new_writer.into();
     }