[Zucchini] Add "-keep" flag; remove redundnat CRC32 check.

By defaulti, Zucchini deletes the output (patch or patched "new") file
upon failure. The "-keep" debug flag overrides this behavior, so the
bad output file can be examined. This CL adds the flag for Zucchini-gen
and Zucchini-apply.

This CL also removes a duplicated call to Ensemble::CheckOldFile(),
which checks the CRC32 of the "old" file. This removal prevents
redundant work and slightly speed up Zucchini-apply (by ~5%).

Bug: 729154
Change-Id: I9ba8e9b00b7783661de06b39fae010761bedc6e6
Reviewed-on: https://chromium-review.googlesource.com/973746
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544786}
NOKEYCHECK=True
GitOrigin-RevId: 21879c3adbfbbf676e5ee28b9f0a19e60d104d80

4 files changed, 31 insertions, 19 deletions

diff --git a/main_utils.cc b/main_utils.cc
index 3a7055a..7a28388 100644
--- a/main_utils.cc
+++ b/main_utils.cc
@@ -63,8 +63,10 @@ struct Command {
 /******** List of Zucchini commands ********/
 
 constexpr Command kCommands[] = {
-    {"gen", "-gen <old_file> <new_file> <patch_file> [-raw]", 3, &MainGen},
-    {"apply", "-apply <old_file> <patch_file> <new_file>", 3, &MainApply},
+    {"gen", "-gen <old_file> <new_file> <patch_file> [-raw] [-keep]", 3,
+     &MainGen},
+    {"apply", "-apply <old_file> <patch_file> <new_file> [-keep]", 3,
+     &MainApply},
     {"read", "-read <exe> [-dump]", 1, &MainRead},
     {"detect", "-detect <archive_file> [-dd=format#]", 1, &MainDetect},
     {"match", "-match <old_file> <new_file>", 2, &MainMatch},
diff --git a/zucchini_commands.cc b/zucchini_commands.cc
index 53b5bd7..2d4b156 100644
--- a/zucchini_commands.cc
+++ b/zucchini_commands.cc
@@ -29,6 +29,7 @@ namespace {
 /******** Command-line Switches ********/
 
 constexpr char kSwitchDump[] = "dump";
+constexpr char kSwitchKeep[] = "keep";
 constexpr char kSwitchRaw[] = "raw";
 
 }  // namespace
@@ -79,6 +80,9 @@ zucchini::status::Code MainGen(MainParams params) {
     return zucchini::status::kStatusFileWriteError;
   }
 
+  if (params.command_line.HasSwitch(kSwitchKeep))
+    patch.Keep();
+
   if (!patch_writer.SerializeInto(patch.region()))
     return zucchini::status::kStatusPatchWriteError;
 
@@ -91,7 +95,8 @@ zucchini::status::Code MainGen(MainParams params) {
 zucchini::status::Code MainApply(MainParams params) {
   CHECK_EQ(3U, params.file_paths.size());
   return zucchini::Apply(params.file_paths[0], params.file_paths[1],
-                         params.file_paths[2]);
+                         params.file_paths[2],
+                         params.command_line.HasSwitch(kSwitchKeep));
 }
 
 zucchini::status::Code MainRead(MainParams params) {
diff --git a/zucchini_integration.cc b/zucchini_integration.cc
index 2d78fdd..1149658 100644
--- a/zucchini_integration.cc
+++ b/zucchini_integration.cc
@@ -42,7 +42,8 @@ struct FileNames {
 status::Code ApplyCommon(base::File&& old_file_handle,
                          base::File&& patch_file_handle,
                          base::File&& new_file_handle,
-                         const FileNames& names) {
+                         const FileNames& names,
+                         bool force_keep) {
   MappedFileReader patch_file(std::move(patch_file_handle));
   if (patch_file.HasError()) {
     LOG(ERROR) << "Error with file " << names.patch_name.value() << ": "
@@ -63,10 +64,6 @@ status::Code ApplyCommon(base::File&& old_file_handle,
                << old_file.error();
     return status::kStatusFileReadError;
   }
-  if (!patch_reader->CheckOldFile(old_file.region())) {
-    LOG(ERROR) << "Invalid old_file.";
-    return status::kStatusInvalidOldImage;
-  }
 
   zucchini::PatchHeader header = patch_reader->header();
   // By default, delete output on destruction, to avoid having lingering files
@@ -83,6 +80,9 @@ status::Code ApplyCommon(base::File&& old_file_handle,
     return status::kStatusFileWriteError;
   }
 
+  if (force_keep)
+    new_file.Keep();
+
   zucchini::status::Code result =
       zucchini::Apply(old_file.region(), *patch_reader, new_file.region());
   if (result != status::kStatusSuccess) {
@@ -100,15 +100,17 @@ status::Code ApplyCommon(base::File&& old_file_handle,
 
 status::Code Apply(base::File&& old_file_handle,
                    base::File&& patch_file_handle,
-                   base::File&& new_file_handle) {
+                   base::File&& new_file_handle,
+                   bool force_keep) {
   const FileNames file_names;
   return ApplyCommon(std::move(old_file_handle), std::move(patch_file_handle),
-                     std::move(new_file_handle), file_names);
+                     std::move(new_file_handle), file_names, force_keep);
 }
 
 status::Code Apply(const base::FilePath& old_path,
                    const base::FilePath& patch_path,
-                   const base::FilePath& new_path) {
+                   const base::FilePath& new_path,
+                   bool force_keep) {
   using base::File;
   File old_file(old_path, File::FLAG_OPEN | File::FLAG_READ);
   File patch_file(patch_path, File::FLAG_OPEN | File::FLAG_READ);
@@ -117,7 +119,7 @@ status::Code Apply(const base::FilePath& old_path,
                               File::FLAG_CAN_DELETE_ON_CLOSE);
   const FileNames file_names(old_path, patch_path, new_path);
   return ApplyCommon(std::move(old_file), std::move(patch_file),
-                     std::move(new_file), file_names);
+                     std::move(new_file), file_names, force_keep);
 }
 
 }  // namespace zucchini
diff --git a/zucchini_integration.h b/zucchini_integration.h
index 7c3fc40..ce98b28 100644
--- a/zucchini_integration.h
+++ b/zucchini_integration.h
@@ -13,21 +13,24 @@ namespace zucchini {
 
 // Applies the patch in |patch_file| to the bytes in |old_file| and writes the
 // result to |new_file|. Since this uses memory mapped files, crashes are
-// expected in case of I/O errors. On Windows |new_file| is kept iff returned
-// code is kStatusSuccess, and is deleted otherwise. For UNIX systems the
-// caller needs to do cleanup since it has ownership of the base::File params
-// and Zucchini has no knowledge of which base::FilePath to delete.
+// expected in case of I/O errors. On Windows, |new_file| is kept iff returned
+// code is kStatusSuccess or if |force_keep == true|, and is deleted otherwise.
+// For UNIX systems the caller needs to do cleanup since it has ownership of the
+// base::File params and Zucchini has no knowledge of which base::FilePath to
+// delete.
 status::Code Apply(base::File&& old_file,
                    base::File&& patch_file,
-                   base::File&& new_file);
+                   base::File&& new_file,
+                   bool force_keep = false);
 
 // Applies the patch in |patch_path| to the bytes in |old_path| and writes the
 // result to |new_path|. Since this uses memory mapped files, crashes are
 // expected in case of I/O errors. |new_path| is kept iff returned code is
-// kStatusSuccess, and is deleted otherwise.
+// kStatusSuccess or if |force_keep == true|, and is deleted otherwise.
 status::Code Apply(const base::FilePath& old_path,
                    const base::FilePath& patch_path,
-                   const base::FilePath& new_path);
+                   const base::FilePath& new_path,
+                   bool force_keep = false);
 
 }  // namespace zucchini