Merge "Log temperature during OTA update" am: bc2c51a97f am: 1d7e500bfc am: 556ca4e301
am: d8df5485eb

Change-Id: Ibbe3bef3e31c7950f0b4d956fad97f6e426bbcaf
diff --git a/Android.mk b/Android.mk
index 58b8a22..037aa16 100644
--- a/Android.mk
+++ b/Android.mk
@@ -29,9 +29,11 @@
 # ===============================
 include $(CLEAR_VARS)
 LOCAL_SRC_FILES := mounts.cpp
-LOCAL_CLANG := true
-LOCAL_CFLAGS := -Wall -Wno-unused-parameter -Werror
+LOCAL_CFLAGS := \
+    -Wall \
+    -Werror
 LOCAL_MODULE := libmounts
+LOCAL_STATIC_LIBRARIES := libbase
 include $(BUILD_STATIC_LIBRARY)
 
 # recovery (static executable)
diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp
index 7be3fdb..51bf393 100644
--- a/applypatch/applypatch.cpp
+++ b/applypatch/applypatch.cpp
@@ -27,6 +27,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include <functional>
 #include <memory>
 #include <string>
 #include <utility>
@@ -42,7 +43,7 @@
 #include "print_sha1.h"
 
 static int LoadPartitionContents(const std::string& filename, FileContents* file);
-static ssize_t FileSink(const unsigned char* data, ssize_t len, void* token);
+static size_t FileSink(const unsigned char* data, size_t len, int fd);
 static int GenerateTarget(const FileContents& source_file, const std::unique_ptr<Value>& patch,
                           const std::string& target_filename,
                           const uint8_t target_sha1[SHA_DIGEST_LENGTH], const Value* bonus_data);
@@ -194,8 +195,8 @@
     return -1;
   }
 
-  ssize_t bytes_written = FileSink(file->data.data(), file->data.size(), &fd);
-  if (bytes_written != static_cast<ssize_t>(file->data.size())) {
+  size_t bytes_written = FileSink(file->data.data(), file->data.size(), fd);
+  if (bytes_written != file->data.size()) {
     printf("short write of \"%s\" (%zd bytes of %zu): %s\n", filename, bytes_written,
            file->data.size(), strerror(errno));
     return -1;
@@ -433,25 +434,17 @@
     return 0;
 }
 
-ssize_t FileSink(const unsigned char* data, ssize_t len, void* token) {
-    int fd = *static_cast<int*>(token);
-    ssize_t done = 0;
-    ssize_t wrote;
-    while (done < len) {
-        wrote = TEMP_FAILURE_RETRY(ota_write(fd, data+done, len-done));
-        if (wrote == -1) {
-            printf("error writing %zd bytes: %s\n", (len-done), strerror(errno));
-            return done;
-        }
-        done += wrote;
+static size_t FileSink(const unsigned char* data, size_t len, int fd) {
+  size_t done = 0;
+  while (done < len) {
+    ssize_t wrote = TEMP_FAILURE_RETRY(ota_write(fd, data + done, len - done));
+    if (wrote == -1) {
+      printf("error writing %zd bytes: %s\n", (len - done), strerror(errno));
+      return done;
     }
-    return done;
-}
-
-ssize_t MemorySink(const unsigned char* data, ssize_t len, void* token) {
-    std::string* s = static_cast<std::string*>(token);
-    s->append(reinterpret_cast<const char*>(data), len);
-    return len;
+    done += wrote;
+  }
+  return done;
 }
 
 // Return the amount of free space (in bytes) on the filesystem
@@ -647,9 +640,11 @@
   }
 
   // We store the decoded output in memory.
-  SinkFn sink = MemorySink;
   std::string memory_sink_str;  // Don't need to reserve space.
-  void* token = &memory_sink_str;
+  SinkFn sink = [&memory_sink_str](const unsigned char* data, size_t len) {
+    memory_sink_str.append(reinterpret_cast<const char*>(data), len);
+    return len;
+  };
 
   SHA_CTX ctx;
   SHA1_Init(&ctx);
@@ -657,10 +652,10 @@
   int result;
   if (use_bsdiff) {
     result = ApplyBSDiffPatch(source_file.data.data(), source_file.data.size(), patch.get(), 0,
-                              sink, token, &ctx);
+                              sink, &ctx);
   } else {
     result = ApplyImagePatch(source_file.data.data(), source_file.data.size(), patch.get(), sink,
-                             token, &ctx, bonus_data);
+                             &ctx, bonus_data);
   }
 
   if (result != 0) {
diff --git a/applypatch/bspatch.cpp b/applypatch/bspatch.cpp
index 9920c2b..f75a2c6 100644
--- a/applypatch/bspatch.cpp
+++ b/applypatch/bspatch.cpp
@@ -24,9 +24,9 @@
 #include <sys/types.h>
 
 #include <bspatch.h>
+#include <openssl/sha.h>
 
 #include "applypatch/applypatch.h"
-#include "openssl/sha.h"
 
 void ShowBSDiffLicense() {
     puts("The bsdiff library used herein is:\n"
@@ -60,10 +60,10 @@
         );
 }
 
-int ApplyBSDiffPatch(const unsigned char* old_data, ssize_t old_size, const Value* patch,
-                     ssize_t patch_offset, SinkFn sink, void* token, SHA_CTX* ctx) {
-  auto sha_sink = [&](const uint8_t* data, size_t len) {
-    len = sink(data, len, token);
+int ApplyBSDiffPatch(const unsigned char* old_data, size_t old_size, const Value* patch,
+                     size_t patch_offset, SinkFn sink, SHA_CTX* ctx) {
+  auto sha_sink = [&sink, &ctx](const uint8_t* data, size_t len) {
+    len = sink(data, len);
     if (ctx) SHA1_Update(ctx, data, len);
     return len;
   };
@@ -72,8 +72,8 @@
                          patch->data.size(), sha_sink);
 }
 
-int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size, const Value* patch,
-                        ssize_t patch_offset, std::vector<unsigned char>* new_data) {
+int ApplyBSDiffPatchMem(const unsigned char* old_data, size_t old_size, const Value* patch,
+                        size_t patch_offset, std::vector<unsigned char>* new_data) {
   auto vector_sink = [new_data](const uint8_t* data, size_t len) {
     new_data->insert(new_data->end(), data, data + len);
     return len;
diff --git a/applypatch/imgpatch.cpp b/applypatch/imgpatch.cpp
index adcc61f..7d8b736 100644
--- a/applypatch/imgpatch.cpp
+++ b/applypatch/imgpatch.cpp
@@ -43,12 +43,11 @@
   return android::base::get_unaligned<int32_t>(address);
 }
 
-int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size,
-                    const unsigned char* patch_data, ssize_t patch_size,
-                    SinkFn sink, void* token) {
+int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const unsigned char* patch_data,
+                    size_t patch_size, SinkFn sink) {
   Value patch(VAL_BLOB, std::string(reinterpret_cast<const char*>(patch_data), patch_size));
 
-  return ApplyImagePatch(old_data, old_size, &patch, sink, token, nullptr, nullptr);
+  return ApplyImagePatch(old_data, old_size, &patch, sink, nullptr, nullptr);
 }
 
 /*
@@ -57,8 +56,8 @@
  * file, and update the SHA context with the output data as well.
  * Return 0 on success.
  */
-int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size, const Value* patch,
-                    SinkFn sink, void* token, SHA_CTX* ctx, const Value* bonus_data) {
+int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const Value* patch, SinkFn sink,
+                    SHA_CTX* ctx, const Value* bonus_data) {
   if (patch->data.size() < 12) {
     printf("patch too short to contain header\n");
     return -1;
@@ -97,11 +96,11 @@
       size_t src_len = static_cast<size_t>(Read8(normal_header + 8));
       size_t patch_offset = static_cast<size_t>(Read8(normal_header + 16));
 
-      if (src_start + src_len > static_cast<size_t>(old_size)) {
+      if (src_start + src_len > old_size) {
         printf("source data too short\n");
         return -1;
       }
-      ApplyBSDiffPatch(old_data + src_start, src_len, patch, patch_offset, sink, token, ctx);
+      ApplyBSDiffPatch(old_data + src_start, src_len, patch, patch_offset, sink, ctx);
     } else if (type == CHUNK_RAW) {
       const char* raw_header = &patch->data[pos];
       pos += 4;
@@ -110,15 +109,14 @@
         return -1;
       }
 
-      ssize_t data_len = Read4(raw_header);
+      size_t data_len = static_cast<size_t>(Read4(raw_header));
 
       if (pos + data_len > patch->data.size()) {
         printf("failed to read chunk %d raw data\n", i);
         return -1;
       }
       if (ctx) SHA1_Update(ctx, &patch->data[pos], data_len);
-      if (sink(reinterpret_cast<const unsigned char*>(&patch->data[pos]), data_len, token) !=
-          data_len) {
+      if (sink(reinterpret_cast<const unsigned char*>(&patch->data[pos]), data_len) != data_len) {
         printf("failed to write chunk %d raw data\n", i);
         return -1;
       }
@@ -143,7 +141,7 @@
       int memLevel = Read4(deflate_header + 52);
       int strategy = Read4(deflate_header + 56);
 
-      if (src_start + src_len > static_cast<size_t>(old_size)) {
+      if (src_start + src_len > old_size) {
         printf("source data too short\n");
         return -1;
       }
@@ -240,9 +238,9 @@
           strm.avail_out = temp_data.size();
           strm.next_out = temp_data.data();
           ret = deflate(&strm, Z_FINISH);
-          ssize_t have = temp_data.size() - strm.avail_out;
+          size_t have = temp_data.size() - strm.avail_out;
 
-          if (sink(temp_data.data(), have, token) != have) {
+          if (sink(temp_data.data(), have) != have) {
             printf("failed to write %zd compressed bytes to output\n", have);
             return -1;
           }
diff --git a/applypatch/include/applypatch/applypatch.h b/applypatch/include/applypatch/applypatch.h
index 4489dec..da55432 100644
--- a/applypatch/include/applypatch/applypatch.h
+++ b/applypatch/include/applypatch/applypatch.h
@@ -20,6 +20,7 @@
 #include <stdint.h>
 #include <sys/stat.h>
 
+#include <functional>
 #include <memory>
 #include <string>
 #include <vector>
@@ -41,7 +42,7 @@
 // and use it as the source instead.
 #define CACHE_TEMP_SOURCE "/cache/saved.file"
 
-typedef ssize_t (*SinkFn)(const unsigned char*, ssize_t, void*);
+using SinkFn = std::function<size_t(const unsigned char*, size_t)>;
 
 // applypatch.cpp
 int ShowLicenses();
@@ -66,18 +67,14 @@
 
 // bspatch.cpp
 void ShowBSDiffLicense();
-int ApplyBSDiffPatch(const unsigned char* old_data, ssize_t old_size,
-                     const Value* patch, ssize_t patch_offset,
-                     SinkFn sink, void* token, SHA_CTX* ctx);
-int ApplyBSDiffPatchMem(const unsigned char* old_data, ssize_t old_size,
-                        const Value* patch, ssize_t patch_offset,
-                        std::vector<unsigned char>* new_data);
+int ApplyBSDiffPatch(const unsigned char* old_data, size_t old_size, const Value* patch,
+                     size_t patch_offset, SinkFn sink, SHA_CTX* ctx);
+int ApplyBSDiffPatchMem(const unsigned char* old_data, size_t old_size, const Value* patch,
+                        size_t patch_offset, std::vector<unsigned char>* new_data);
 
 // imgpatch.cpp
-int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size,
-                    const Value* patch,
-                    SinkFn sink, void* token, SHA_CTX* ctx,
-                    const Value* bonus_data);
+int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const Value* patch, SinkFn sink,
+                    SHA_CTX* ctx, const Value* bonus_data);
 
 // freecache.cpp
 int MakeFreeSpaceOnCache(size_t bytes_needed);
diff --git a/applypatch/include/applypatch/imgpatch.h b/applypatch/include/applypatch/imgpatch.h
index 6549f79..07c6609 100644
--- a/applypatch/include/applypatch/imgpatch.h
+++ b/applypatch/include/applypatch/imgpatch.h
@@ -19,10 +19,11 @@
 
 #include <sys/types.h>
 
-using SinkFn = ssize_t (*)(const unsigned char*, ssize_t, void*);
+#include <functional>
 
-int ApplyImagePatch(const unsigned char* old_data, ssize_t old_size,
-                    const unsigned char* patch_data, ssize_t patch_size,
-                    SinkFn sink, void* token);
+using SinkFn = std::function<size_t(const unsigned char*, size_t)>;
+
+int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const unsigned char* patch_data,
+                    size_t patch_size, SinkFn sink);
 
 #endif  // _APPLYPATCH_IMGPATCH_H
diff --git a/edify/parser.yy b/edify/parser.yy
index 97205fe..b1685eb 100644
--- a/edify/parser.yy
+++ b/edify/parser.yy
@@ -23,6 +23,8 @@
 #include <string>
 #include <vector>
 
+#include <android-base/macros.h>
+
 #include "expr.h"
 #include "yydefs.h"
 #include "parser.h"
@@ -121,6 +123,7 @@
     $$->emplace_back($1);
 }
 | arglist ',' expr {
+    UNUSED($1);
     $$->push_back(std::unique_ptr<Expr>($3));
 }
 ;
diff --git a/minadbd/Android.mk b/minadbd/Android.mk
index 7eef13e..de0b0c8 100644
--- a/minadbd/Android.mk
+++ b/minadbd/Android.mk
@@ -29,6 +29,7 @@
 
 LOCAL_CLANG := true
 LOCAL_MODULE := minadbd_test
+LOCAL_COMPATIBILITY_SUITE := device-tests
 LOCAL_SRC_FILES := fuse_adb_provider_test.cpp
 LOCAL_CFLAGS := $(minadbd_cflags)
 LOCAL_C_INCLUDES := $(LOCAL_PATH) system/core/adb
diff --git a/minadbd/AndroidTest.xml b/minadbd/AndroidTest.xml
new file mode 100644
index 0000000..7ea235b
--- /dev/null
+++ b/minadbd/AndroidTest.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2017 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+<configuration description="Config for minadbd_test">
+    <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
+        <option name="cleanup" value="true" />
+        <option name="push" value="minadbd_test->/data/local/tmp/minadbd_test" />
+    </target_preparer>
+    <option name="test-suite-tag" value="apct" />
+    <test class="com.android.tradefed.testtype.GTest" >
+        <option name="native-test-device-path" value="/data/local/tmp" />
+        <option name="module-name" value="minadbd_test" />
+    </test>
+</configuration>
\ No newline at end of file
diff --git a/mounts.cpp b/mounts.cpp
index f23376b..fbcbac0 100644
--- a/mounts.cpp
+++ b/mounts.cpp
@@ -27,6 +27,8 @@
 #include <string>
 #include <vector>
 
+#include <android-base/logging.h>
+
 struct MountedVolume {
     std::string device;
     std::string mount_point;
@@ -75,15 +77,23 @@
 }
 
 int unmount_mounted_volume(MountedVolume* volume) {
-    // Intentionally pass the empty string to umount if the caller tries
-    // to unmount a volume they already unmounted using this
-    // function.
-    std::string mount_point = volume->mount_point;
-    volume->mount_point.clear();
-    return umount(mount_point.c_str());
+  // Intentionally pass the empty string to umount if the caller tries to unmount a volume they
+  // already unmounted using this function.
+  std::string mount_point = volume->mount_point;
+  volume->mount_point.clear();
+  int result = umount(mount_point.c_str());
+  if (result == -1) {
+    PLOG(WARNING) << "Failed to umount " << mount_point;
+  }
+  return result;
 }
 
 int remount_read_only(MountedVolume* volume) {
-    return mount(volume->device.c_str(), volume->mount_point.c_str(), volume->filesystem.c_str(),
-                 MS_NOATIME | MS_NODEV | MS_NODIRATIME | MS_RDONLY | MS_REMOUNT, 0);
+  int result = mount(volume->device.c_str(), volume->mount_point.c_str(),
+                     volume->filesystem.c_str(),
+                     MS_NOATIME | MS_NODEV | MS_NODIRATIME | MS_RDONLY | MS_REMOUNT, 0);
+  if (result == -1) {
+    PLOG(WARNING) << "Failed to remount read-only " << volume->mount_point;
+  }
+  return result;
 }
diff --git a/tests/Android.mk b/tests/Android.mk
index ff6e14c..80eae8f 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -20,7 +20,7 @@
 include $(CLEAR_VARS)
 LOCAL_CFLAGS := -Werror
 LOCAL_MODULE := recovery_unit_test
-LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
+LOCAL_COMPATIBILITY_SUITE := device-tests
 LOCAL_STATIC_LIBRARIES := \
     libverifier \
     libminui \
@@ -45,10 +45,8 @@
 
 # Manual tests
 include $(CLEAR_VARS)
-LOCAL_CLANG := true
 LOCAL_CFLAGS := -Werror
 LOCAL_MODULE := recovery_manual_test
-LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
 LOCAL_STATIC_LIBRARIES := \
     libminui \
     libbase
@@ -85,8 +83,8 @@
     -Werror \
     -D_FILE_OFFSET_BITS=64
 
-LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
 LOCAL_MODULE := recovery_component_test
+LOCAL_COMPATIBILITY_SUITE := device-tests
 LOCAL_C_INCLUDES := bootable/recovery
 LOCAL_SRC_FILES := \
     component/applypatch_test.cpp \
diff --git a/tests/AndroidTest.xml b/tests/AndroidTest.xml
new file mode 100644
index 0000000..3999aa5
--- /dev/null
+++ b/tests/AndroidTest.xml
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2017 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+<configuration description="Config for recovery_component_test and recovery_unit_test">
+    <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
+        <option name="cleanup" value="true" />
+        <option name="push" value="recovery_component_test->/data/local/tmp/recovery_component_test" />
+        <option name="push" value="recovery_unit_test->/data/local/tmp/recovery_unit_test" />
+    </target_preparer>
+    <option name="test-suite-tag" value="apct" />
+    <test class="com.android.tradefed.testtype.GTest" >
+        <option name="native-test-device-path" value="/data/local/tmp" />
+        <option name="module-name" value="recovery_component_test" />
+    </test>
+    <test class="com.android.tradefed.testtype.GTest" >
+        <option name="native-test-device-path" value="/data/local/tmp" />
+        <option name="module-name" value="recovery_unit_test" />
+    </test>
+</configuration>
diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp
index 2f64850..7d00a3d 100644
--- a/tests/component/imgdiff_test.cpp
+++ b/tests/component/imgdiff_test.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <stdio.h>
+
 #include <string>
 #include <vector>
 
@@ -27,12 +29,6 @@
 
 using android::base::get_unaligned;
 
-static ssize_t MemorySink(const unsigned char* data, ssize_t len, void* token) {
-  std::string* s = static_cast<std::string*>(token);
-  s->append(reinterpret_cast<const char*>(data), len);
-  return len;
-}
-
 // Sanity check for the given imgdiff patch header.
 static void verify_patch_header(const std::string& patch, size_t* num_normal, size_t* num_raw,
                                 size_t* num_deflate) {
@@ -79,6 +75,18 @@
   if (num_deflate != nullptr) *num_deflate = deflate;
 }
 
+static void verify_patched_image(const std::string& src, const std::string& patch,
+                                 const std::string& tgt) {
+  std::string patched;
+  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
+                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
+                               [&patched](const unsigned char* data, size_t len) {
+                                 patched.append(reinterpret_cast<const char*>(data), len);
+                                 return len;
+                               }));
+  ASSERT_EQ(tgt, patched);
+}
+
 TEST(ImgdiffTest, invalid_args) {
   // Insufficient inputs.
   ASSERT_EQ(2, imgdiff(1, (const char* []){ "imgdiff" }));
@@ -124,11 +132,7 @@
   ASSERT_EQ(0U, num_deflate);
   ASSERT_EQ(1U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, zip_mode_smoke_store) {
@@ -177,11 +181,7 @@
   ASSERT_EQ(0U, num_deflate);
   ASSERT_EQ(1U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, zip_mode_smoke_compressed) {
@@ -230,11 +230,7 @@
   ASSERT_EQ(1U, num_deflate);
   ASSERT_EQ(2U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, zip_mode_smoke_trailer_zeros) {
@@ -286,11 +282,7 @@
   ASSERT_EQ(1U, num_deflate);
   ASSERT_EQ(2U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, image_mode_simple) {
@@ -333,11 +325,7 @@
   ASSERT_EQ(1U, num_deflate);
   ASSERT_EQ(2U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, image_mode_different_num_chunks) {
@@ -413,11 +401,7 @@
   ASSERT_EQ(1U, num_deflate);
   ASSERT_EQ(2U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, image_mode_spurious_magic) {
@@ -454,11 +438,7 @@
   ASSERT_EQ(0U, num_deflate);
   ASSERT_EQ(1U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, image_mode_short_input1) {
@@ -494,11 +474,7 @@
   ASSERT_EQ(0U, num_deflate);
   ASSERT_EQ(1U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, image_mode_short_input2) {
@@ -534,11 +510,7 @@
   ASSERT_EQ(0U, num_deflate);
   ASSERT_EQ(1U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
 
 TEST(ImgdiffTest, image_mode_single_entry_long) {
@@ -577,9 +549,5 @@
   ASSERT_EQ(0U, num_deflate);
   ASSERT_EQ(0U, num_raw);
 
-  std::string patched;
-  ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast<const unsigned char*>(src.data()), src.size(),
-                               reinterpret_cast<const unsigned char*>(patch.data()), patch.size(),
-                               MemorySink, &patched));
-  ASSERT_EQ(tgt, patched);
+  verify_patched_image(src, patch, tgt);
 }
diff --git a/tests/component/uncrypt_test.cpp b/tests/component/uncrypt_test.cpp
index 4f2b816..5e057e1 100644
--- a/tests/component/uncrypt_test.cpp
+++ b/tests/component/uncrypt_test.cpp
@@ -25,12 +25,15 @@
 #include <android-base/file.h>
 #include <android-base/logging.h>
 #include <android-base/properties.h>
+#include <android-base/test_utils.h>
 #include <android-base/unique_fd.h>
 #include <bootloader_message/bootloader_message.h>
 #include <gtest/gtest.h>
 
 #include "common/component_test_util.h"
 
+using namespace std::string_literals;
+
 static const std::string UNCRYPT_SOCKET = "/dev/socket/uncrypt";
 static const std::string INIT_SVC_SETUP_BCB = "init.svc.setup-bcb";
 static const std::string INIT_SVC_CLEAR_BCB = "init.svc.clear-bcb";
@@ -65,128 +68,104 @@
     has_misc = parse_misc();
   }
 
+  void SetupOrClearBcb(bool isSetup, const std::string& message,
+                       const std::string& message_in_bcb) const {
+    if (!has_misc) {
+      GTEST_LOG_(INFO) << "Test skipped due to no /misc partition found on the device.";
+      return;
+    }
+
+    // Trigger the setup-bcb service.
+    ASSERT_TRUE(android::base::SetProperty("ctl.start", isSetup ? "setup-bcb" : "clear-bcb"));
+
+    // Test tends to be flaky if proceeding immediately ("Transport endpoint is not connected").
+    sleep(1);
+
+    sockaddr_un un = {};
+    un.sun_family = AF_UNIX;
+    strlcpy(un.sun_path, UNCRYPT_SOCKET.c_str(), sizeof(un.sun_path));
+
+    int sockfd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+    ASSERT_NE(-1, sockfd);
+
+    // Connect to the uncrypt socket.
+    bool success = false;
+    for (int retry = 0; retry < SOCKET_CONNECTION_MAX_RETRY; retry++) {
+      if (connect(sockfd, reinterpret_cast<sockaddr*>(&un), sizeof(sockaddr_un)) != 0) {
+        success = true;
+        break;
+      }
+      sleep(1);
+    }
+    ASSERT_TRUE(success);
+
+    if (isSetup) {
+      // Send out the BCB message.
+      int length = static_cast<int>(message.size());
+      int length_out = htonl(length);
+      ASSERT_TRUE(android::base::WriteFully(sockfd, &length_out, sizeof(int)))
+          << "Failed to write length: " << strerror(errno);
+      ASSERT_TRUE(android::base::WriteFully(sockfd, message.data(), length))
+          << "Failed to write message: " << strerror(errno);
+    }
+
+    // Check the status code from uncrypt.
+    int status;
+    ASSERT_TRUE(android::base::ReadFully(sockfd, &status, sizeof(int)));
+    ASSERT_EQ(100U, ntohl(status));
+
+    // Ack having received the status code.
+    int code = 0;
+    ASSERT_TRUE(android::base::WriteFully(sockfd, &code, sizeof(int)));
+
+    ASSERT_EQ(0, close(sockfd));
+
+    ASSERT_TRUE(android::base::SetProperty("ctl.stop", isSetup ? "setup-bcb" : "clear-bcb"));
+
+    // Verify the message by reading from BCB directly.
+    bootloader_message boot;
+    std::string err;
+    ASSERT_TRUE(read_bootloader_message(&boot, &err)) << "Failed to read BCB: " << err;
+
+    if (isSetup) {
+      ASSERT_EQ("boot-recovery", std::string(boot.command));
+      ASSERT_EQ(message_in_bcb, std::string(boot.recovery));
+
+      // The rest of the boot.recovery message should be zero'd out.
+      ASSERT_LE(message_in_bcb.size(), sizeof(boot.recovery));
+      size_t left = sizeof(boot.recovery) - message_in_bcb.size();
+      ASSERT_EQ(std::string(left, '\0'), std::string(&boot.recovery[message_in_bcb.size()], left));
+
+      // Clear the BCB.
+      ASSERT_TRUE(clear_bootloader_message(&err)) << "Failed to clear BCB: " << err;
+    } else {
+      // All the bytes should be cleared.
+      ASSERT_EQ(std::string(sizeof(boot), '\0'),
+                std::string(reinterpret_cast<const char*>(&boot), sizeof(boot)));
+    }
+  }
+
   bool has_misc;
 };
 
 TEST_F(UncryptTest, setup_bcb) {
-  if (!has_misc) {
-    GTEST_LOG_(INFO) << "Test skipped due to no /misc partition found on the device.";
-    return;
-  }
-
-  // Trigger the setup-bcb service.
-  ASSERT_TRUE(android::base::SetProperty("ctl.start", "setup-bcb"));
-
-  // Test tends to be flaky if proceeding immediately ("Transport endpoint is not connected").
-  sleep(1);
-
-  struct sockaddr_un un = {};
-  un.sun_family = AF_UNIX;
-  strlcpy(un.sun_path, UNCRYPT_SOCKET.c_str(), sizeof(un.sun_path));
-
-  int sockfd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
-  ASSERT_NE(-1, sockfd);
-
-  // Connect to the uncrypt socket.
-  bool success = false;
-  for (int retry = 0; retry < SOCKET_CONNECTION_MAX_RETRY; retry++) {
-    if (connect(sockfd, reinterpret_cast<struct sockaddr*>(&un), sizeof(struct sockaddr_un)) != 0) {
-      success = true;
-      break;
-    }
-    sleep(1);
-  }
-  ASSERT_TRUE(success);
-
-  // Send out the BCB message.
   std::string message = "--update_message=abc value";
   std::string message_in_bcb = "recovery\n--update_message=abc value\n";
-  int length = static_cast<int>(message.size());
-  int length_out = htonl(length);
-  ASSERT_TRUE(android::base::WriteFully(sockfd, &length_out, sizeof(int)))
-      << "Failed to write length: " << strerror(errno);
-  ASSERT_TRUE(android::base::WriteFully(sockfd, message.data(), length))
-      << "Failed to write message: " << strerror(errno);
-
-  // Check the status code from uncrypt.
-  int status;
-  ASSERT_TRUE(android::base::ReadFully(sockfd, &status, sizeof(int)));
-  ASSERT_EQ(100U, ntohl(status));
-
-  // Ack having received the status code.
-  int code = 0;
-  ASSERT_TRUE(android::base::WriteFully(sockfd, &code, sizeof(int)));
-
-  ASSERT_EQ(0, close(sockfd));
-
-  ASSERT_TRUE(android::base::SetProperty("ctl.stop", "setup-bcb"));
-
-  // Verify the message by reading from BCB directly.
-  bootloader_message boot;
-  std::string err;
-  ASSERT_TRUE(read_bootloader_message(&boot, &err)) << "Failed to read BCB: " << err;
-
-  ASSERT_EQ("boot-recovery", std::string(boot.command));
-  ASSERT_EQ(message_in_bcb, std::string(boot.recovery));
-
-  // The rest of the boot.recovery message should be zero'd out.
-  ASSERT_LE(message_in_bcb.size(), sizeof(boot.recovery));
-  size_t left = sizeof(boot.recovery) - message_in_bcb.size();
-  ASSERT_EQ(std::string(left, '\0'), std::string(&boot.recovery[message_in_bcb.size()], left));
-
-  // Clear the BCB.
-  ASSERT_TRUE(clear_bootloader_message(&err)) << "Failed to clear BCB: " << err;
+  SetupOrClearBcb(true, message, message_in_bcb);
 }
 
 TEST_F(UncryptTest, clear_bcb) {
-  if (!has_misc) {
-    GTEST_LOG_(INFO) << "Test skipped due to no /misc partition found on the device.";
-    return;
-  }
+  SetupOrClearBcb(false, "", "");
+}
 
-  // Trigger the clear-bcb service.
-  ASSERT_TRUE(android::base::SetProperty("ctl.start", "clear-bcb"));
+TEST_F(UncryptTest, setup_bcb_wipe_ab) {
+  TemporaryFile wipe_package;
+  ASSERT_TRUE(android::base::WriteStringToFile(std::string(345, 'a'), wipe_package.path));
 
-  // Test tends to be flaky if proceeding immediately ("Transport endpoint is not connected").
-  sleep(1);
-
-  struct sockaddr_un un = {};
-  un.sun_family = AF_UNIX;
-  strlcpy(un.sun_path, UNCRYPT_SOCKET.c_str(), sizeof(un.sun_path));
-
-  int sockfd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
-  ASSERT_NE(-1, sockfd);
-
-  // Connect to the uncrypt socket.
-  bool success = false;
-  for (int retry = 0; retry < SOCKET_CONNECTION_MAX_RETRY; retry++) {
-    if (connect(sockfd, reinterpret_cast<struct sockaddr*>(&un), sizeof(struct sockaddr_un)) != 0) {
-      success = true;
-      break;
-    }
-    sleep(1);
-  }
-  ASSERT_TRUE(success);
-
-  // Check the status code from uncrypt.
-  int status;
-  ASSERT_TRUE(android::base::ReadFully(sockfd, &status, sizeof(int)));
-  ASSERT_EQ(100U, ntohl(status));
-
-  // Ack having received the status code.
-  int code = 0;
-  ASSERT_TRUE(android::base::WriteFully(sockfd, &code, sizeof(int)));
-
-  ASSERT_EQ(0, close(sockfd));
-
-  ASSERT_TRUE(android::base::SetProperty("ctl.stop", "clear-bcb"));
-
-  // Verify the content by reading from BCB directly.
-  bootloader_message boot;
-  std::string err;
-  ASSERT_TRUE(read_bootloader_message(&boot, &err)) << "Failed to read BCB: " << err;
-
-  // All the bytes should be cleared.
-  ASSERT_EQ(std::string(sizeof(boot), '\0'),
-            std::string(reinterpret_cast<const char*>(&boot), sizeof(boot)));
+  // It's expected to store a wipe package in /misc, with the package size passed to recovery.
+  std::string message =
+      "--wipe_ab\n--wipe_package="s + wipe_package.path + "\n--reason=wipePackage"s;
+  std::string message_in_bcb =
+      "recovery\n--wipe_ab\n--wipe_package_size=345\n--reason=wipePackage\n";
+  SetupOrClearBcb(true, message, message_in_bcb);
 }
diff --git a/tests/component/verifier_test.cpp b/tests/component/verifier_test.cpp
index 4c06487..61ceadc 100644
--- a/tests/component/verifier_test.cpp
+++ b/tests/component/verifier_test.cpp
@@ -132,6 +132,51 @@
                                         package.size(), certs));
 }
 
+TEST(VerifierTest, BadPackage_AlteredFooter) {
+  std::string testkey_v3;
+  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3));
+  TemporaryFile key_file1;
+  ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file1.path));
+  std::vector<Certificate> certs;
+  ASSERT_TRUE(load_keys(key_file1.path, certs));
+
+  std::string package;
+  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("otasigned_v3.zip"), &package));
+  ASSERT_EQ(std::string("\xc0\x06\xff\xff\xd2\x06", 6), package.substr(package.size() - 6, 6));
+
+  // Alter the footer.
+  package[package.size() - 5] = '\x05';
+  ASSERT_EQ(VERIFY_FAILURE,
+            verify_file(reinterpret_cast<const unsigned char*>(package.data()), package.size(),
+                        certs));
+}
+
+TEST(VerifierTest, BadPackage_AlteredContent) {
+  std::string testkey_v3;
+  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("testkey_v3.txt"), &testkey_v3));
+  TemporaryFile key_file1;
+  ASSERT_TRUE(android::base::WriteStringToFile(testkey_v3, key_file1.path));
+  std::vector<Certificate> certs;
+  ASSERT_TRUE(load_keys(key_file1.path, certs));
+
+  std::string package;
+  ASSERT_TRUE(android::base::ReadFileToString(from_testdata_base("otasigned_v3.zip"), &package));
+  ASSERT_GT(package.size(), static_cast<size_t>(100));
+
+  // Alter the content.
+  std::string altered1(package);
+  altered1[50] += 1;
+  ASSERT_EQ(VERIFY_FAILURE,
+            verify_file(reinterpret_cast<const unsigned char*>(altered1.data()), altered1.size(),
+                        certs));
+
+  std::string altered2(package);
+  altered2[10] += 1;
+  ASSERT_EQ(VERIFY_FAILURE,
+            verify_file(reinterpret_cast<const unsigned char*>(altered2.data()), altered2.size(),
+                        certs));
+}
+
 TEST_P(VerifierSuccessTest, VerifySucceed) {
   ASSERT_EQ(verify_file(memmap.addr, memmap.length, certs, nullptr), VERIFY_SUCCESS);
 }
@@ -174,6 +219,4 @@
 INSTANTIATE_TEST_CASE_P(BadPackage, VerifierFailureTest,
     ::testing::Values(
       std::vector<std::string>({"random.zip", "v1"}),
-      std::vector<std::string>({"fake-eocd.zip", "v1"}),
-      std::vector<std::string>({"alter-metadata.zip", "v1"}),
-      std::vector<std::string>({"alter-footer.zip", "v1"})));
+      std::vector<std::string>({"fake-eocd.zip", "v1"})));
diff --git a/tests/testdata/alter-footer.zip b/tests/testdata/alter-footer.zip
deleted file mode 100644
index f497ec0..0000000
--- a/tests/testdata/alter-footer.zip
+++ /dev/null
Binary files differ
diff --git a/tests/testdata/alter-metadata.zip b/tests/testdata/alter-metadata.zip
deleted file mode 100644
index 1c71fbc..0000000
--- a/tests/testdata/alter-metadata.zip
+++ /dev/null
Binary files differ
diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp
index c614ccc..a1a5773 100644
--- a/updater/blockimg.cpp
+++ b/updater/blockimg.cpp
@@ -18,6 +18,7 @@
 #include <errno.h>
 #include <dirent.h>
 #include <fcntl.h>
+#include <inttypes.h>
 #include <linux/fs.h>
 #include <pthread.h>
 #include <stdarg.h>
@@ -231,128 +232,135 @@
     buffer.resize(size);
 }
 
-struct RangeSinkState {
-    explicit RangeSinkState(RangeSet& rs) : tgt(rs) { };
+/**
+ * RangeSinkWriter reads data from the given FD, and writes them to the destination specified by the
+ * given RangeSet.
+ */
+class RangeSinkWriter {
+ public:
+  RangeSinkWriter(int fd, const RangeSet& tgt)
+      : fd_(fd), tgt_(tgt), next_range_(0), current_range_left_(0) {
+    CHECK_NE(tgt.count, static_cast<size_t>(0));
+  };
 
-    int fd;
-    const RangeSet& tgt;
-    size_t p_block;
-    size_t p_remain;
-};
+  bool Finished() const {
+    return next_range_ == tgt_.count && current_range_left_ == 0;
+  }
 
-static ssize_t RangeSinkWrite(const uint8_t* data, ssize_t size, void* token) {
-    RangeSinkState* rss = reinterpret_cast<RangeSinkState*>(token);
-
-    if (rss->p_remain == 0) {
-        LOG(ERROR) << "range sink write overrun";
-        return 0;
+  size_t Write(const uint8_t* data, size_t size) {
+    if (Finished()) {
+      LOG(ERROR) << "range sink write overrun; can't write " << size << " bytes";
+      return 0;
     }
 
-    ssize_t written = 0;
+    size_t written = 0;
     while (size > 0) {
-        size_t write_now = size;
-
-        if (rss->p_remain < write_now) {
-            write_now = rss->p_remain;
-        }
-
-        if (write_all(rss->fd, data, write_now) == -1) {
+      // Move to the next range as needed.
+      if (current_range_left_ == 0) {
+        if (next_range_ < tgt_.count) {
+          off64_t offset = static_cast<off64_t>(tgt_.pos[next_range_ * 2]) * BLOCKSIZE;
+          current_range_left_ =
+              (tgt_.pos[next_range_ * 2 + 1] - tgt_.pos[next_range_ * 2]) * BLOCKSIZE;
+          next_range_++;
+          if (!discard_blocks(fd_, offset, current_range_left_)) {
             break;
+          }
+
+          if (!check_lseek(fd_, offset, SEEK_SET)) {
+            break;
+          }
+        } else {
+          // We can't write any more; return how many bytes have been written so far.
+          break;
         }
+      }
 
-        data += write_now;
-        size -= write_now;
+      size_t write_now = size;
+      if (current_range_left_ < write_now) {
+        write_now = current_range_left_;
+      }
 
-        rss->p_remain -= write_now;
-        written += write_now;
+      if (write_all(fd_, data, write_now) == -1) {
+        break;
+      }
 
-        if (rss->p_remain == 0) {
-            // move to the next block
-            ++rss->p_block;
-            if (rss->p_block < rss->tgt.count) {
-                rss->p_remain = (rss->tgt.pos[rss->p_block * 2 + 1] -
-                                 rss->tgt.pos[rss->p_block * 2]) * BLOCKSIZE;
+      data += write_now;
+      size -= write_now;
 
-                off64_t offset = static_cast<off64_t>(rss->tgt.pos[rss->p_block*2]) * BLOCKSIZE;
-                if (!discard_blocks(rss->fd, offset, rss->p_remain)) {
-                    break;
-                }
-
-                if (!check_lseek(rss->fd, offset, SEEK_SET)) {
-                    break;
-                }
-
-            } else {
-                // we can't write any more; return how many bytes have
-                // been written so far.
-                break;
-            }
-        }
+      current_range_left_ -= write_now;
+      written += write_now;
     }
 
     return written;
-}
+  }
 
-// All of the data for all the 'new' transfers is contained in one
-// file in the update package, concatenated together in the order in
-// which transfers.list will need it.  We want to stream it out of the
-// archive (it's compressed) without writing it to a temp file, but we
-// can't write each section until it's that transfer's turn to go.
-//
-// To achieve this, we expand the new data from the archive in a
-// background thread, and block that threads 'receive uncompressed
-// data' function until the main thread has reached a point where we
-// want some new data to be written.  We signal the background thread
-// with the destination for the data and block the main thread,
-// waiting for the background thread to complete writing that section.
-// Then it signals the main thread to wake up and goes back to
-// blocking waiting for a transfer.
-//
-// NewThreadInfo is the struct used to pass information back and forth
-// between the two threads.  When the main thread wants some data
-// written, it sets rss to the destination location and signals the
-// condition.  When the background thread is done writing, it clears
-// rss and signals the condition again.
+ private:
+  // The input data.
+  int fd_;
+  // The destination for the data.
+  const RangeSet& tgt_;
+  // The next range that we should write to.
+  size_t next_range_;
+  // The number of bytes to write before moving to the next range.
+  size_t current_range_left_;
+};
 
+/**
+ * All of the data for all the 'new' transfers is contained in one file in the update package,
+ * concatenated together in the order in which transfers.list will need it. We want to stream it out
+ * of the archive (it's compressed) without writing it to a temp file, but we can't write each
+ * section until it's that transfer's turn to go.
+ *
+ * To achieve this, we expand the new data from the archive in a background thread, and block that
+ * threads 'receive uncompressed data' function until the main thread has reached a point where we
+ * want some new data to be written. We signal the background thread with the destination for the
+ * data and block the main thread, waiting for the background thread to complete writing that
+ * section. Then it signals the main thread to wake up and goes back to blocking waiting for a
+ * transfer.
+ *
+ * NewThreadInfo is the struct used to pass information back and forth between the two threads. When
+ * the main thread wants some data written, it sets writer to the destination location and signals
+ * the condition. When the background thread is done writing, it clears writer and signals the
+ * condition again.
+ */
 struct NewThreadInfo {
-    ZipArchiveHandle za;
-    ZipEntry entry;
+  ZipArchiveHandle za;
+  ZipEntry entry;
 
-    RangeSinkState* rss;
+  RangeSinkWriter* writer;
 
-    pthread_mutex_t mu;
-    pthread_cond_t cv;
+  pthread_mutex_t mu;
+  pthread_cond_t cv;
 };
 
 static bool receive_new_data(const uint8_t* data, size_t size, void* cookie) {
-    NewThreadInfo* nti = reinterpret_cast<NewThreadInfo*>(cookie);
+  NewThreadInfo* nti = static_cast<NewThreadInfo*>(cookie);
 
-    while (size > 0) {
-        // Wait for nti->rss to be non-null, indicating some of this
-        // data is wanted.
-        pthread_mutex_lock(&nti->mu);
-        while (nti->rss == nullptr) {
-            pthread_cond_wait(&nti->cv, &nti->mu);
-        }
-        pthread_mutex_unlock(&nti->mu);
-
-        // At this point nti->rss is set, and we own it.  The main
-        // thread is waiting for it to disappear from nti.
-        ssize_t written = RangeSinkWrite(data, size, nti->rss);
-        data += written;
-        size -= written;
-
-        if (nti->rss->p_block == nti->rss->tgt.count) {
-            // we have written all the bytes desired by this rss.
-
-            pthread_mutex_lock(&nti->mu);
-            nti->rss = nullptr;
-            pthread_cond_broadcast(&nti->cv);
-            pthread_mutex_unlock(&nti->mu);
-        }
+  while (size > 0) {
+    // Wait for nti->writer to be non-null, indicating some of this data is wanted.
+    pthread_mutex_lock(&nti->mu);
+    while (nti->writer == nullptr) {
+      pthread_cond_wait(&nti->cv, &nti->mu);
     }
+    pthread_mutex_unlock(&nti->mu);
 
-    return true;
+    // At this point nti->writer is set, and we own it. The main thread is waiting for it to
+    // disappear from nti.
+    size_t written = nti->writer->Write(data, size);
+    data += written;
+    size -= written;
+
+    if (nti->writer->Finished()) {
+      // We have written all the bytes desired by this writer.
+
+      pthread_mutex_lock(&nti->mu);
+      nti->writer = nullptr;
+      pthread_cond_broadcast(&nti->cv);
+      pthread_mutex_unlock(&nti->mu);
+    }
+  }
+
+  return true;
 }
 
 static void* unzip_new_data(void* cookie) {
@@ -383,28 +391,26 @@
 }
 
 static int WriteBlocks(const RangeSet& tgt, const std::vector<uint8_t>& buffer, int fd) {
-    const uint8_t* data = buffer.data();
-
-    size_t p = 0;
-    for (size_t i = 0; i < tgt.count; ++i) {
-        off64_t offset = static_cast<off64_t>(tgt.pos[i * 2]) * BLOCKSIZE;
-        size_t size = (tgt.pos[i * 2 + 1] - tgt.pos[i * 2]) * BLOCKSIZE;
-        if (!discard_blocks(fd, offset, size)) {
-            return -1;
-        }
-
-        if (!check_lseek(fd, offset, SEEK_SET)) {
-            return -1;
-        }
-
-        if (write_all(fd, data + p, size) == -1) {
-            return -1;
-        }
-
-        p += size;
+  size_t written = 0;
+  for (size_t i = 0; i < tgt.count; ++i) {
+    off64_t offset = static_cast<off64_t>(tgt.pos[i * 2]) * BLOCKSIZE;
+    size_t size = (tgt.pos[i * 2 + 1] - tgt.pos[i * 2]) * BLOCKSIZE;
+    if (!discard_blocks(fd, offset, size)) {
+      return -1;
     }
 
-    return 0;
+    if (!check_lseek(fd, offset, SEEK_SET)) {
+      return -1;
+    }
+
+    if (write_all(fd, buffer.data() + written, size) == -1) {
+      return -1;
+    }
+
+    written += size;
+  }
+
+  return 0;
 }
 
 // Parameters for transfer list command functions
@@ -696,7 +702,7 @@
 }
 
 static int WriteStash(const std::string& base, const std::string& id, int blocks,
-                      std::vector<uint8_t>& buffer, bool checkspace, bool *exists) {
+                      std::vector<uint8_t>& buffer, bool checkspace, bool* exists) {
     if (base.empty()) {
         return -1;
     }
@@ -883,96 +889,81 @@
     }
 }
 
-// Do a source/target load for move/bsdiff/imgdiff in version 2.
-// We expect to parse the remainder of the parameter tokens as one of:
-//
-//    <tgt_range> <src_block_count> <src_range>
-//        (loads data from source image only)
-//
-//    <tgt_range> <src_block_count> - <[stash_id:stash_range] ...>
-//        (loads data from stashes only)
-//
-//    <tgt_range> <src_block_count> <src_range> <src_loc> <[stash_id:stash_range] ...>
-//        (loads data from both source image and stashes)
-//
-// On return, params.buffer is filled with the loaded source data (rearranged and combined with
-// stashed data as necessary). buffer may be reallocated if needed to accommodate the source data.
-// *tgt is the target RangeSet. Any stashes required are loaded using LoadStash.
+/**
+ * We expect to parse the remainder of the parameter tokens as one of:
+ *
+ *    <src_block_count> <src_range>
+ *        (loads data from source image only)
+ *
+ *    <src_block_count> - <[stash_id:stash_range] ...>
+ *        (loads data from stashes only)
+ *
+ *    <src_block_count> <src_range> <src_loc> <[stash_id:stash_range] ...>
+ *        (loads data from both source image and stashes)
+ *
+ * On return, params.buffer is filled with the loaded source data (rearranged and combined with
+ * stashed data as necessary). buffer may be reallocated if needed to accommodate the source data.
+ * tgt is the target RangeSet for detecting overlaps. Any stashes required are loaded using
+ * LoadStash.
+ */
+static int LoadSourceBlocks(CommandParameters& params, const RangeSet& tgt, size_t* src_blocks,
+                            bool* overlap) {
+  CHECK(src_blocks != nullptr);
+  CHECK(overlap != nullptr);
 
-static int LoadSrcTgtVersion2(CommandParameters& params, RangeSet& tgt, size_t& src_blocks,
-                              bool* overlap) {
+  // <src_block_count>
+  const std::string& token = params.tokens[params.cpos++];
+  if (!android::base::ParseUint(token, src_blocks)) {
+    LOG(ERROR) << "invalid src_block_count \"" << token << "\"";
+    return -1;
+  }
 
-    // At least it needs to provide three parameters: <tgt_range>,
-    // <src_block_count> and "-"/<src_range>.
-    if (params.cpos + 2 >= params.tokens.size()) {
-        LOG(ERROR) << "invalid parameters";
-        return -1;
+  allocate(*src_blocks * BLOCKSIZE, params.buffer);
+
+  // "-" or <src_range> [<src_loc>]
+  if (params.tokens[params.cpos] == "-") {
+    // no source ranges, only stashes
+    params.cpos++;
+  } else {
+    RangeSet src = parse_range(params.tokens[params.cpos++]);
+    *overlap = range_overlaps(src, tgt);
+
+    if (ReadBlocks(src, params.buffer, params.fd) == -1) {
+      return -1;
     }
 
-    // <tgt_range>
-    tgt = parse_range(params.tokens[params.cpos++]);
-
-    // <src_block_count>
-    const std::string& token = params.tokens[params.cpos++];
-    if (!android::base::ParseUint(token.c_str(), &src_blocks)) {
-        LOG(ERROR) << "invalid src_block_count \"" << token << "\"";
-        return -1;
+    if (params.cpos >= params.tokens.size()) {
+      // no stashes, only source range
+      return 0;
     }
 
-    allocate(src_blocks * BLOCKSIZE, params.buffer);
+    RangeSet locs = parse_range(params.tokens[params.cpos++]);
+    MoveRange(params.buffer, locs, params.buffer);
+  }
 
-    // "-" or <src_range> [<src_loc>]
-    if (params.tokens[params.cpos] == "-") {
-        // no source ranges, only stashes
-        params.cpos++;
-    } else {
-        RangeSet src = parse_range(params.tokens[params.cpos++]);
-        int res = ReadBlocks(src, params.buffer, params.fd);
-
-        if (overlap) {
-            *overlap = range_overlaps(src, tgt);
-        }
-
-        if (res == -1) {
-            return -1;
-        }
-
-        if (params.cpos >= params.tokens.size()) {
-            // no stashes, only source range
-            return 0;
-        }
-
-        RangeSet locs = parse_range(params.tokens[params.cpos++]);
-        MoveRange(params.buffer, locs, params.buffer);
+  // <[stash_id:stash_range]>
+  while (params.cpos < params.tokens.size()) {
+    // Each word is a an index into the stash table, a colon, and then a RangeSet describing where
+    // in the source block that stashed data should go.
+    std::vector<std::string> tokens = android::base::Split(params.tokens[params.cpos++], ":");
+    if (tokens.size() != 2) {
+      LOG(ERROR) << "invalid parameter";
+      return -1;
     }
 
-    // <[stash_id:stash_range]>
-    while (params.cpos < params.tokens.size()) {
-        // Each word is a an index into the stash table, a colon, and
-        // then a rangeset describing where in the source block that
-        // stashed data should go.
-        std::vector<std::string> tokens = android::base::Split(params.tokens[params.cpos++], ":");
-        if (tokens.size() != 2) {
-            LOG(ERROR) << "invalid parameter";
-            return -1;
-        }
-
-        std::vector<uint8_t> stash;
-        int res = LoadStash(params, tokens[0], false, nullptr, stash, true);
-
-        if (res == -1) {
-            // These source blocks will fail verification if used later, but we
-            // will let the caller decide if this is a fatal failure
-            LOG(ERROR) << "failed to load stash " << tokens[0];
-            continue;
-        }
-
-        RangeSet locs = parse_range(tokens[1]);
-
-        MoveRange(params.buffer, locs, stash);
+    std::vector<uint8_t> stash;
+    if (LoadStash(params, tokens[0], false, nullptr, stash, true) == -1) {
+      // These source blocks will fail verification if used later, but we
+      // will let the caller decide if this is a fatal failure
+      LOG(ERROR) << "failed to load stash " << tokens[0];
+      continue;
     }
 
-    return 0;
+    RangeSet locs = parse_range(tokens[1]);
+    MoveRange(params.buffer, locs, stash);
+  }
+
+  return 0;
 }
 
 /**
@@ -989,9 +980,8 @@
  *    <tgt_range> <src_block_count> <src_range> <src_loc> <[stash_id:stash_range] ...>
  *        (loads data from both source image and stashes)
  *
- * Parameters are the same as for LoadSrcTgtVersion2, except for 'onehash', which tells the function
- * whether to expect separate source and targe block hashes, or if they are both the same and only
- * one hash should be expected, and 'isunresumable', which receives a non-zero value if block
+ * 'onehash' tells whether to expect separate source and targe block hashes, or if they are both the
+ * same and only one hash should be expected. params.isunresumable will be set to true if block
  * verification fails in a way that the update cannot be resumed anymore.
  *
  * If the function is unable to load the necessary blocks or their contents don't match the hashes,
@@ -1002,87 +992,100 @@
  *
  * If the return value is 0, source blocks have expected content and the command can be performed.
  */
-static int LoadSrcTgtVersion3(CommandParameters& params, RangeSet& tgt, size_t& src_blocks,
-                              bool onehash, bool& overlap) {
-    if (params.cpos >= params.tokens.size()) {
-        LOG(ERROR) << "missing source hash";
-        return -1;
-    }
+static int LoadSrcTgtVersion3(CommandParameters& params, RangeSet& tgt, size_t* src_blocks,
+                              bool onehash, bool* overlap) {
+  CHECK(src_blocks != nullptr);
+  CHECK(overlap != nullptr);
 
-    std::string srchash = params.tokens[params.cpos++];
-    std::string tgthash;
-
-    if (onehash) {
-        tgthash = srchash;
-    } else {
-        if (params.cpos >= params.tokens.size()) {
-            LOG(ERROR) << "missing target hash";
-            return -1;
-        }
-        tgthash = params.tokens[params.cpos++];
-    }
-
-    if (LoadSrcTgtVersion2(params, tgt, src_blocks, &overlap) == -1) {
-        return -1;
-    }
-
-    std::vector<uint8_t> tgtbuffer(tgt.size * BLOCKSIZE);
-
-    if (ReadBlocks(tgt, tgtbuffer, params.fd) == -1) {
-        return -1;
-    }
-
-    if (VerifyBlocks(tgthash, tgtbuffer, tgt.size, false) == 0) {
-        // Target blocks already have expected content, command should be skipped.
-        return 1;
-    }
-
-    if (VerifyBlocks(srchash, params.buffer, src_blocks, true) == 0) {
-        // If source and target blocks overlap, stash the source blocks so we can
-        // resume from possible write errors. In verify mode, we can skip stashing
-        // because the source blocks won't be overwritten.
-        if (overlap && params.canwrite) {
-            LOG(INFO) << "stashing " << src_blocks << " overlapping blocks to " << srchash;
-
-            bool stash_exists = false;
-            if (WriteStash(params.stashbase, srchash, src_blocks, params.buffer, true,
-                           &stash_exists) != 0) {
-                LOG(ERROR) << "failed to stash overlapping source blocks";
-                return -1;
-            }
-
-            params.stashed += src_blocks;
-            // Can be deleted when the write has completed.
-            if (!stash_exists) {
-                params.freestash = srchash;
-            }
-        }
-
-        // Source blocks have expected content, command can proceed.
-        return 0;
-    }
-
-    if (overlap && LoadStash(params, srchash, true, nullptr, params.buffer, true) == 0) {
-        // Overlapping source blocks were previously stashed, command can proceed.
-        // We are recovering from an interrupted command, so we don't know if the
-        // stash can safely be deleted after this command.
-        return 0;
-    }
-
-    // Valid source data not available, update cannot be resumed.
-    LOG(ERROR) << "partition has unexpected contents";
-    PrintHashForCorruptedSourceBlocks(params, params.buffer);
-
-    params.isunresumable = true;
-
+  if (params.cpos >= params.tokens.size()) {
+    LOG(ERROR) << "missing source hash";
     return -1;
+  }
+
+  std::string srchash = params.tokens[params.cpos++];
+  std::string tgthash;
+
+  if (onehash) {
+    tgthash = srchash;
+  } else {
+    if (params.cpos >= params.tokens.size()) {
+      LOG(ERROR) << "missing target hash";
+      return -1;
+    }
+    tgthash = params.tokens[params.cpos++];
+  }
+
+  // At least it needs to provide three parameters: <tgt_range>, <src_block_count> and
+  // "-"/<src_range>.
+  if (params.cpos + 2 >= params.tokens.size()) {
+    LOG(ERROR) << "invalid parameters";
+    return -1;
+  }
+
+  // <tgt_range>
+  tgt = parse_range(params.tokens[params.cpos++]);
+
+  std::vector<uint8_t> tgtbuffer(tgt.size * BLOCKSIZE);
+  if (ReadBlocks(tgt, tgtbuffer, params.fd) == -1) {
+    return -1;
+  }
+
+  // Return now if target blocks already have expected content.
+  if (VerifyBlocks(tgthash, tgtbuffer, tgt.size, false) == 0) {
+    return 1;
+  }
+
+  // Load source blocks.
+  if (LoadSourceBlocks(params, tgt, src_blocks, overlap) == -1) {
+    return -1;
+  }
+
+  if (VerifyBlocks(srchash, params.buffer, *src_blocks, true) == 0) {
+    // If source and target blocks overlap, stash the source blocks so we can
+    // resume from possible write errors. In verify mode, we can skip stashing
+    // because the source blocks won't be overwritten.
+    if (*overlap && params.canwrite) {
+      LOG(INFO) << "stashing " << *src_blocks << " overlapping blocks to " << srchash;
+
+      bool stash_exists = false;
+      if (WriteStash(params.stashbase, srchash, *src_blocks, params.buffer, true,
+                     &stash_exists) != 0) {
+        LOG(ERROR) << "failed to stash overlapping source blocks";
+        return -1;
+      }
+
+      params.stashed += *src_blocks;
+      // Can be deleted when the write has completed.
+      if (!stash_exists) {
+        params.freestash = srchash;
+      }
+    }
+
+    // Source blocks have expected content, command can proceed.
+    return 0;
+  }
+
+  if (*overlap && LoadStash(params, srchash, true, nullptr, params.buffer, true) == 0) {
+    // Overlapping source blocks were previously stashed, command can proceed. We are recovering
+    // from an interrupted command, so we don't know if the stash can safely be deleted after this
+    // command.
+    return 0;
+  }
+
+  // Valid source data not available, update cannot be resumed.
+  LOG(ERROR) << "partition has unexpected contents";
+  PrintHashForCorruptedSourceBlocks(params, params.buffer);
+
+  params.isunresumable = true;
+
+  return -1;
 }
 
 static int PerformCommandMove(CommandParameters& params) {
   size_t blocks = 0;
   bool overlap = false;
   RangeSet tgt;
-  int status = LoadSrcTgtVersion3(params, tgt, blocks, true, overlap);
+  int status = LoadSrcTgtVersion3(params, tgt, &blocks, true, &overlap);
 
   if (status == -1) {
     LOG(ERROR) << "failed to read blocks for move";
@@ -1220,134 +1223,111 @@
 }
 
 static int PerformCommandNew(CommandParameters& params) {
+  if (params.cpos >= params.tokens.size()) {
+    LOG(ERROR) << "missing target blocks for new";
+    return -1;
+  }
 
-    if (params.cpos >= params.tokens.size()) {
-        LOG(ERROR) << "missing target blocks for new";
-        return -1;
+  RangeSet tgt = parse_range(params.tokens[params.cpos++]);
+
+  if (params.canwrite) {
+    LOG(INFO) << " writing " << tgt.size << " blocks of new data";
+
+    RangeSinkWriter writer(params.fd, tgt);
+    pthread_mutex_lock(&params.nti.mu);
+    params.nti.writer = &writer;
+    pthread_cond_broadcast(&params.nti.cv);
+
+    while (params.nti.writer != nullptr) {
+      pthread_cond_wait(&params.nti.cv, &params.nti.mu);
     }
 
-    RangeSet tgt = parse_range(params.tokens[params.cpos++]);
+    pthread_mutex_unlock(&params.nti.mu);
+  }
 
-    if (params.canwrite) {
-        LOG(INFO) << " writing " << tgt.size << " blocks of new data";
+  params.written += tgt.size;
 
-        RangeSinkState rss(tgt);
-        rss.fd = params.fd;
-        rss.p_block = 0;
-        rss.p_remain = (tgt.pos[1] - tgt.pos[0]) * BLOCKSIZE;
-
-        off64_t offset = static_cast<off64_t>(tgt.pos[0]) * BLOCKSIZE;
-        if (!discard_blocks(params.fd, offset, tgt.size * BLOCKSIZE)) {
-            return -1;
-        }
-
-        if (!check_lseek(params.fd, offset, SEEK_SET)) {
-            return -1;
-        }
-
-        pthread_mutex_lock(&params.nti.mu);
-        params.nti.rss = &rss;
-        pthread_cond_broadcast(&params.nti.cv);
-
-        while (params.nti.rss) {
-            pthread_cond_wait(&params.nti.cv, &params.nti.mu);
-        }
-
-        pthread_mutex_unlock(&params.nti.mu);
-    }
-
-    params.written += tgt.size;
-
-    return 0;
+  return 0;
 }
 
 static int PerformCommandDiff(CommandParameters& params) {
+  // <offset> <length>
+  if (params.cpos + 1 >= params.tokens.size()) {
+    LOG(ERROR) << "missing patch offset or length for " << params.cmdname;
+    return -1;
+  }
 
-    // <offset> <length>
-    if (params.cpos + 1 >= params.tokens.size()) {
-        LOG(ERROR) << "missing patch offset or length for " << params.cmdname;
-        return -1;
-    }
+  size_t offset;
+  if (!android::base::ParseUint(params.tokens[params.cpos++], &offset)) {
+    LOG(ERROR) << "invalid patch offset";
+    return -1;
+  }
 
-    size_t offset;
-    if (!android::base::ParseUint(params.tokens[params.cpos++].c_str(), &offset)) {
-        LOG(ERROR) << "invalid patch offset";
-        return -1;
-    }
+  size_t len;
+  if (!android::base::ParseUint(params.tokens[params.cpos++], &len)) {
+    LOG(ERROR) << "invalid patch len";
+    return -1;
+  }
 
-    size_t len;
-    if (!android::base::ParseUint(params.tokens[params.cpos++].c_str(), &len)) {
-        LOG(ERROR) << "invalid patch len";
-        return -1;
-    }
+  RangeSet tgt;
+  size_t blocks = 0;
+  bool overlap = false;
+  int status = LoadSrcTgtVersion3(params, tgt, &blocks, false, &overlap);
 
-    RangeSet tgt;
-    size_t blocks = 0;
-    bool overlap = false;
-    int status = LoadSrcTgtVersion3(params, tgt, blocks, false, overlap);
+  if (status == -1) {
+    LOG(ERROR) << "failed to read blocks for diff";
+    return -1;
+  }
 
-    if (status == -1) {
-        LOG(ERROR) << "failed to read blocks for diff";
-        return -1;
-    }
+  if (status == 0) {
+    params.foundwrites = true;
+  } else if (params.foundwrites) {
+    LOG(WARNING) << "warning: commands executed out of order [" << params.cmdname << "]";
+  }
 
+  if (params.canwrite) {
     if (status == 0) {
-        params.foundwrites = true;
-    } else if (params.foundwrites) {
-        LOG(WARNING) << "warning: commands executed out of order [" << params.cmdname << "]";
-    }
+      LOG(INFO) << "patching " << blocks << " blocks to " << tgt.size;
+      Value patch_value(
+          VAL_BLOB, std::string(reinterpret_cast<const char*>(params.patch_start + offset), len));
 
-    if (params.canwrite) {
-        if (status == 0) {
-            LOG(INFO) << "patching " << blocks << " blocks to " << tgt.size;
-            Value patch_value(VAL_BLOB,
-                    std::string(reinterpret_cast<const char*>(params.patch_start + offset), len));
-            RangeSinkState rss(tgt);
-            rss.fd = params.fd;
-            rss.p_block = 0;
-            rss.p_remain = (tgt.pos[1] - tgt.pos[0]) * BLOCKSIZE;
-
-            off64_t offset = static_cast<off64_t>(tgt.pos[0]) * BLOCKSIZE;
-            if (!discard_blocks(params.fd, offset, rss.p_remain)) {
-                return -1;
-            }
-
-            if (!check_lseek(params.fd, offset, SEEK_SET)) {
-                return -1;
-            }
-
-            if (params.cmdname[0] == 'i') {      // imgdiff
-                if (ApplyImagePatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value,
-                        &RangeSinkWrite, &rss, nullptr, nullptr) != 0) {
-                    LOG(ERROR) << "Failed to apply image patch.";
-                    return -1;
-                }
-            } else {
-                if (ApplyBSDiffPatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value,
-                        0, &RangeSinkWrite, &rss, nullptr) != 0) {
-                    LOG(ERROR) << "Failed to apply bsdiff patch.";
-                    return -1;
-                }
-            }
-
-            // We expect the output of the patcher to fill the tgt ranges exactly.
-            if (rss.p_block != tgt.count || rss.p_remain != 0) {
-                LOG(ERROR) << "range sink underrun?";
-            }
-        } else {
-            LOG(INFO) << "skipping " << blocks << " blocks already patched to " << tgt.size
-                      << " [" << params.cmdline << "]";
+      RangeSinkWriter writer(params.fd, tgt);
+      if (params.cmdname[0] == 'i') {  // imgdiff
+        if (ApplyImagePatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value,
+                            std::bind(&RangeSinkWriter::Write, &writer, std::placeholders::_1,
+                                      std::placeholders::_2),
+                            nullptr, nullptr) != 0) {
+          LOG(ERROR) << "Failed to apply image patch.";
+          return -1;
         }
+      } else {
+        if (ApplyBSDiffPatch(params.buffer.data(), blocks * BLOCKSIZE, &patch_value, 0,
+                             std::bind(&RangeSinkWriter::Write, &writer, std::placeholders::_1,
+                                       std::placeholders::_2),
+                             nullptr) != 0) {
+          LOG(ERROR) << "Failed to apply bsdiff patch.";
+          return -1;
+        }
+      }
+
+      // We expect the output of the patcher to fill the tgt ranges exactly.
+      if (!writer.Finished()) {
+        LOG(ERROR) << "range sink underrun?";
+      }
+    } else {
+      LOG(INFO) << "skipping " << blocks << " blocks already patched to " << tgt.size << " ["
+                << params.cmdline << "]";
     }
+  }
 
-    if (!params.freestash.empty()) {
-        FreeStash(params.stashbase, params.freestash);
-        params.freestash.clear();
-    }
+  if (!params.freestash.empty()) {
+    FreeStash(params.stashbase, params.freestash);
+    params.freestash.clear();
+  }
 
-    params.written += tgt.size;
+  params.written += tgt.size;
 
-    return 0;
+  return 0;
 }
 
 static int PerformCommandErase(CommandParameters& params) {
@@ -1834,7 +1814,7 @@
     uint16_t mount_count = *reinterpret_cast<uint16_t*>(&block0_buffer[0x400+0x34]);
 
     if (mount_count > 0) {
-        uiPrintf(state, "Device was remounted R/W %d times\n", mount_count);
+        uiPrintf(state, "Device was remounted R/W %" PRIu16 " times", mount_count);
         uiPrintf(state, "Last remount happened on %s", ctime(&mount_time));
     }
 
@@ -1871,7 +1851,7 @@
     LOG(INFO) << filename->data << " image corrupted, attempting to recover...";
 
     // When opened with O_RDWR, libfec rewrites corrupted blocks when they are read
-    fec::io fh(filename->data.c_str(), O_RDWR);
+    fec::io fh(filename->data, O_RDWR);
 
     if (!fh) {
         ErrorAbort(state, kLibfecFailure, "fec_open \"%s\" failed: %s", filename->data.c_str(),
diff --git a/updater/install.cpp b/updater/install.cpp
index f91f3fc..857d7f1 100644
--- a/updater/install.cpp
+++ b/updater/install.cpp
@@ -181,8 +181,8 @@
 
   if (mount(location.c_str(), mount_point.c_str(), fs_type.c_str(),
             MS_NOATIME | MS_NODEV | MS_NODIRATIME, mount_options.c_str()) < 0) {
-    uiPrintf(state, "%s: failed to mount %s at %s: %s\n", name, location.c_str(),
-             mount_point.c_str(), strerror(errno));
+    uiPrintf(state, "%s: Failed to mount %s at %s: %s", name, location.c_str(), mount_point.c_str(),
+             strerror(errno));
     return StringValue("");
   }
 
@@ -231,12 +231,12 @@
   scan_mounted_volumes();
   MountedVolume* vol = find_mounted_volume_by_mount_point(mount_point.c_str());
   if (vol == nullptr) {
-    uiPrintf(state, "unmount of %s failed; no such volume\n", mount_point.c_str());
+    uiPrintf(state, "Failed to unmount %s: No such volume", mount_point.c_str());
     return nullptr;
   } else {
     int ret = unmount_mounted_volume(vol);
     if (ret != 0) {
-      uiPrintf(state, "unmount of %s failed (%d): %s\n", mount_point.c_str(), ret, strerror(errno));
+      uiPrintf(state, "Failed to unmount %s: %s", mount_point.c_str(), strerror(errno));
     }
   }
 
@@ -699,15 +699,15 @@
   return StringValue(result == 0 ? "t" : "");
 }
 
-// This is the updater side handler for ui_print() in edify script. Contents
-// will be sent over to the recovery side for on-screen display.
+// This is the updater side handler for ui_print() in edify script. Contents will be sent over to
+// the recovery side for on-screen display.
 Value* UIPrintFn(const char* name, State* state, const std::vector<std::unique_ptr<Expr>>& argv) {
   std::vector<std::string> args;
   if (!ReadArgs(state, argv, &args)) {
-    return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name);
+    return ErrorAbort(state, kArgsParsingFailure, "%s(): Failed to parse the argument(s)", name);
   }
 
-  std::string buffer = android::base::Join(args, "") + "\n";
+  std::string buffer = android::base::Join(args, "");
   uiPrint(state, buffer);
   return StringValue(buffer);
 }