commit bcb15aaae8ce567c940837ff564c904c4733cfcd
author Sen Jiang <senj@google.com> Wed Feb 07 18:01:48 2018 -0800
committer Sen Jiang <senj@google.com> Thu Feb 08 21:55:11 2018 +0000
tree 8df52eba4d3e4a53005f1a08fae0a52099253e87
parent 7d2800522c9cbc134c13dc920119d5eb1aa2acb9

Fix End() not called on ExtentWriter.

Pass the DirectExtentWriter as a pointer to CommonHashExtents to avoid
constructing the class if we are not using it.

Bug: 73085863
Test: brillo_update_payload verify a delta payload
Change-Id: Ie76aa1b533cafc7d649fd55316d9b1c6066822ba

payload_consumer/file_descriptor_utils.cc

diff --git a/payload_consumer/file_descriptor_utils.cc b/payload_consumer/file_descriptor_utils.cc
index 4ffded2..b1902de 100644
--- a/payload_consumer/file_descriptor_utils.cc
+++ b/payload_consumer/file_descriptor_utils.cc
@@ -37,8 +37,7 @@
 
 bool CommonHashExtents(FileDescriptorPtr source,
                        const RepeatedPtrField<Extent>& src_extents,
-                       FileDescriptorPtr target,
-                       const RepeatedPtrField<Extent>& tgt_extents,
+                       DirectExtentWriter* writer,
                        uint64_t block_size,
                        brillo::Blob* hash_out) {
   auto total_blocks = utils::BlocksInExtents(src_extents);
@@ -50,11 +49,6 @@
 
   DirectExtentReader reader;
   TEST_AND_RETURN_FALSE(reader.Init(source, src_extents, block_size));
-  DirectExtentWriter writer;
-  if (target) {
-    TEST_AND_RETURN_FALSE(writer.Init(target, tgt_extents, block_size));
-    TEST_AND_RETURN_FALSE(total_blocks == utils::BlocksInExtents(tgt_extents));
-  }
 
   HashCalculator source_hasher;
   while (total_blocks > 0) {
@@ -64,14 +58,12 @@
       TEST_AND_RETURN_FALSE(
           source_hasher.Update(buf.data(), read_blocks * block_size));
     }
-    if (target) {
-      TEST_AND_RETURN_FALSE(writer.Write(buf.data(), read_blocks * block_size));
+    if (writer) {
+      TEST_AND_RETURN_FALSE(
+          writer->Write(buf.data(), read_blocks * block_size));
     }
     total_blocks -= read_blocks;
   }
-  if (target) {
-    TEST_AND_RETURN_FALSE(writer.End());
-  }
 
   if (hash_out != nullptr) {
     TEST_AND_RETURN_FALSE(source_hasher.Finalize());
@@ -90,9 +82,13 @@
                         const RepeatedPtrField<Extent>& tgt_extents,
                         uint64_t block_size,
                         brillo::Blob* hash_out) {
-  TEST_AND_RETURN_FALSE(target);
-  TEST_AND_RETURN_FALSE(CommonHashExtents(
-      source, src_extents, target, tgt_extents, block_size, hash_out));
+  DirectExtentWriter writer;
+  TEST_AND_RETURN_FALSE(writer.Init(target, tgt_extents, block_size));
+  TEST_AND_RETURN_FALSE(utils::BlocksInExtents(src_extents) ==
+                        utils::BlocksInExtents(tgt_extents));
+  TEST_AND_RETURN_FALSE(
+      CommonHashExtents(source, src_extents, &writer, block_size, hash_out));
+  TEST_AND_RETURN_FALSE(writer.End());
   return true;
 }
 
@@ -102,7 +98,7 @@
                         brillo::Blob* hash_out) {
   TEST_AND_RETURN_FALSE(hash_out != nullptr);
   TEST_AND_RETURN_FALSE(
-      CommonHashExtents(source, extents, nullptr, {}, block_size, hash_out));
+      CommonHashExtents(source, extents, nullptr, block_size, hash_out));
   return true;
 }