Merge changes I2d42f55a,Ic1b5dbf7

am: b5fe2dd00c

Change-Id: I253e8f580cfa41a6a240cb311970a3165fd69fb0

install/adb_install.cpp

@@ -90,7 +90,7 @@ static bool WriteStatusToFd(MinadbdCommandStatus status, int fd) {
 
 // Installs the package from FUSE. Returns the installation result and whether it should continue
 // waiting for new commands.
-static auto AdbInstallPackageHandler(RecoveryUI* ui, int* result) {
+static auto AdbInstallPackageHandler(RecoveryUI* ui, InstallResult* result) {
   // How long (in seconds) we wait for the package path to be ready. It doesn't need to be too long
   // because the minadbd service has already issued an install command. FUSE_SIDELOAD_HOST_PATHNAME
   // will start to exist once the host connects and starts serving a package. Poll for its
@@ -110,7 +110,7 @@ static auto AdbInstallPackageHandler(RecoveryUI* ui, int* result) {
         break;
       }
     }
-    *result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0, ui);
+    *result = InstallPackage(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0, ui);
     break;
   }
 
@@ -120,7 +120,7 @@ static auto AdbInstallPackageHandler(RecoveryUI* ui, int* result) {
   return std::make_pair(*result == INSTALL_SUCCESS, should_continue);
 }
 
-static auto AdbRebootHandler(MinadbdCommand command, int* result,
+static auto AdbRebootHandler(MinadbdCommand command, InstallResult* result,
                              Device::BuiltinAction* reboot_action) {
   // Use Device::REBOOT_{FASTBOOT,RECOVERY,RESCUE}, instead of the ones with ENTER_. This allows
   // rebooting back into fastboot/recovery/rescue mode through bootloader, which may use a newly
@@ -331,7 +331,7 @@ static void CreateMinadbdServiceAndExecuteCommands(
   signal(SIGPIPE, SIG_DFL);
 }
 
-int ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action) {
+InstallResult ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action) {
   // Save the usb state to restore after the sideload operation.
   std::string usb_state = android::base::GetProperty("sys.usb.state", "none");
   // Clean up state and stop adbd.
@@ -342,7 +342,7 @@ int ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot
 
   RecoveryUI* ui = device->GetUI();
 
-  int install_result = INSTALL_ERROR;
+  InstallResult install_result = INSTALL_ERROR;
   std::map<MinadbdCommand, CommandFunction> command_map{
     { MinadbdCommand::kInstall, std::bind(&AdbInstallPackageHandler, ui, &install_result) },
     { MinadbdCommand::kRebootAndroid, std::bind(&AdbRebootHandler, MinadbdCommand::kRebootAndroid,

install/fuse_sdcard_install.cpp

@@ -133,7 +133,7 @@ static bool StartSdcardFuse(const std::string& path) {
   return run_fuse_sideload(std::move(file_data_reader)) == 0;
 }
 
-int ApplyFromSdcard(Device* device, RecoveryUI* ui) {
+InstallResult ApplyFromSdcard(Device* device, RecoveryUI* ui) {
   if (ensure_path_mounted(SDCARD_ROOT) != 0) {
     LOG(ERROR) << "\n-- Couldn't mount " << SDCARD_ROOT << ".\n";
     return INSTALL_ERROR;
@@ -159,9 +159,8 @@ int ApplyFromSdcard(Device* device, RecoveryUI* ui) {
     _exit(status ? EXIT_SUCCESS : EXIT_FAILURE);
   }
 
-  // FUSE_SIDELOAD_HOST_PATHNAME will start to exist once the fuse in child
+  // FUSE_SIDELOAD_HOST_PATHNAME will start to exist once the fuse in child process is ready.
-  // process is ready.
+  InstallResult result = INSTALL_ERROR;
-  int result = INSTALL_ERROR;
   int status;
   bool waited = false;
   for (int i = 0; i < SDCARD_INSTALL_TIMEOUT; ++i) {
@@ -184,7 +183,7 @@ int ApplyFromSdcard(Device* device, RecoveryUI* ui) {
       }
     }
 
-    result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0 /*retry_count*/, ui);
+    result = InstallPackage(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0 /* retry_count */, ui);
     break;
   }
 

install/include/install/adb_install.h

@@ -16,9 +16,10 @@
 
 #pragma once
 
-#include <recovery_ui/device.h>
+#include "install/install.h"
+#include "recovery_ui/device.h"
 
-// Applies a package via `adb sideload` or `adb rescue`. Returns the install result (in `enum
+// Applies a package via `adb sideload` or `adb rescue`. Returns the install result. When a reboot
-// InstallResult`). When a reboot has been requested, INSTALL_REBOOT will be the return value, with
+// has been requested, INSTALL_REBOOT will be the return value, with the reboot target set in
-// the reboot target set in reboot_action.
+// reboot_action.
-int ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action);
+InstallResult ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action);

install/include/install/fuse_sdcard_install.h

@@ -16,7 +16,8 @@
 
 #pragma once
 
+#include "install/install.h"
 #include "recovery_ui/device.h"
 #include "recovery_ui/ui.h"
 
-int ApplyFromSdcard(Device* device, RecoveryUI* ui);
+InstallResult ApplyFromSdcard(Device* device, RecoveryUI* ui);

install/include/install/install.h

@@ -47,8 +47,8 @@ enum class OtaType {
 // Installs the given update package. This function should also wipe the cache partition after a
 // successful installation if |should_wipe_cache| is true or an updater command asks to wipe the
 // cache.
-int install_package(const std::string& package, bool should_wipe_cache, bool needs_mount,
+InstallResult InstallPackage(const std::string& package, bool should_wipe_cache, bool needs_mount,
-                    int retry_count, RecoveryUI* ui);
+                             int retry_count, RecoveryUI* ui);
 
 // Verifies the package by ota keys. Returns true if the package is verified successfully,
 // otherwise returns false.
@@ -62,7 +62,7 @@ bool ReadMetadataFromPackage(ZipArchiveHandle zip, std::map<std::string, std::st
 // entry doesn't exist.
 bool verify_package_compatibility(ZipArchiveHandle package_zip);
 
-// Checks if the the metadata in the OTA package has expected values. Returns 0 on success.
+// Checks if the metadata in the OTA package has expected values. Mandatory checks: ota-type,
-// Mandatory checks: ota-type, pre-device and serial number(if presents)
+// pre-device and serial number (if presents). A/B OTA specific checks: pre-build version,
-// AB OTA specific checks: pre-build version, fingerprint, timestamp.
+// fingerprint, timestamp.
-int CheckPackageMetadata(const std::map<std::string, std::string>& metadata, OtaType ota_type);
+bool CheckPackageMetadata(const std::map<std::string, std::string>& metadata, OtaType ota_type);

install/include/private/setup_commands.h

@@ -27,13 +27,13 @@
 // |zip| located at |package|. Stores the command line that should be called into |cmd|. The
 // |status_fd| is the file descriptor the child process should use to report back the progress of
 // the update.
-int SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int retry_count,
+bool SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int retry_count,
-                             int status_fd, std::vector<std::string>* cmd);
+                              int status_fd, std::vector<std::string>* cmd);
 
 // Sets up the commands for an A/B update. Extracts the needed entries from the open zip archive
 // |zip| located at |package|. Stores the command line that should be called into |cmd|. The
 // |status_fd| is the file descriptor the child process should use to report back the progress of
 // the update. Note that since this applies to the sideloading flow only, it takes one less
 // parameter |retry_count| than the non-A/B version.
-int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd,
+bool SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd,
-                          std::vector<std::string>* cmd);
+                           std::vector<std::string>* cmd);

install/install.cpp

@@ -139,14 +139,14 @@ static void ReadSourceTargetBuild(const std::map<std::string, std::string>& meta
 // Checks the build version, fingerprint and timestamp in the metadata of the A/B package.
 // Downgrading is not allowed unless explicitly enabled in the package and only for
 // incremental packages.
-static int CheckAbSpecificMetadata(const std::map<std::string, std::string>& metadata) {
+static bool CheckAbSpecificMetadata(const std::map<std::string, std::string>& metadata) {
   // Incremental updates should match the current build.
   auto device_pre_build = android::base::GetProperty("ro.build.version.incremental", "");
   auto pkg_pre_build = get_value(metadata, "pre-build-incremental");
   if (!pkg_pre_build.empty() && pkg_pre_build != device_pre_build) {
     LOG(ERROR) << "Package is for source build " << pkg_pre_build << " but expected "
                << device_pre_build;
-    return INSTALL_ERROR;
+    return false;
   }
 
   auto device_fingerprint = android::base::GetProperty("ro.build.fingerprint", "");
@@ -154,7 +154,7 @@ static int CheckAbSpecificMetadata(const std::map<std::string, std::string>& met
   if (!pkg_pre_build_fingerprint.empty() && pkg_pre_build_fingerprint != device_fingerprint) {
     LOG(ERROR) << "Package is for source build " << pkg_pre_build_fingerprint << " but expected "
                << device_fingerprint;
-    return INSTALL_ERROR;
+    return false;
   }
 
   // Check for downgrade version.
@@ -172,36 +172,36 @@ static int CheckAbSpecificMetadata(const std::map<std::string, std::string>& met
                     "newer than timestamp "
                  << build_timestamp << " but package has timestamp " << pkg_post_timestamp
                  << " and downgrade not allowed.";
-      return INSTALL_ERROR;
+      return false;
     }
     if (pkg_pre_build_fingerprint.empty()) {
       LOG(ERROR) << "Downgrade package must have a pre-build version set, not allowed.";
-      return INSTALL_ERROR;
+      return false;
     }
   }
 
-  return 0;
+  return true;
 }
 
-int CheckPackageMetadata(const std::map<std::string, std::string>& metadata, OtaType ota_type) {
+bool CheckPackageMetadata(const std::map<std::string, std::string>& metadata, OtaType ota_type) {
   auto package_ota_type = get_value(metadata, "ota-type");
   auto expected_ota_type = OtaTypeToString(ota_type);
   if (ota_type != OtaType::AB && ota_type != OtaType::BRICK) {
     LOG(INFO) << "Skip package metadata check for ota type " << expected_ota_type;
-    return 0;
+    return true;
   }
 
   if (package_ota_type != expected_ota_type) {
     LOG(ERROR) << "Unexpected ota package type, expects " << expected_ota_type << ", actual "
                << package_ota_type;
-    return INSTALL_ERROR;
+    return false;
   }
 
   auto device = android::base::GetProperty("ro.product.device", "");
   auto pkg_device = get_value(metadata, "pre-device");
   if (pkg_device != device || pkg_device.empty()) {
     LOG(ERROR) << "Package is for product " << pkg_device << " but expected " << device;
-    return INSTALL_ERROR;
+    return false;
   }
 
   // We allow the package to not have any serialno; and we also allow it to carry multiple serial
@@ -218,7 +218,7 @@ int CheckPackageMetadata(const std::map<std::string, std::string>& metadata, Ota
     }
     if (!serial_number_match) {
       LOG(ERROR) << "Package is for serial " << pkg_serial_no;
-      return INSTALL_ERROR;
+      return false;
     }
   }
 
@@ -226,11 +226,11 @@ int CheckPackageMetadata(const std::map<std::string, std::string>& metadata, Ota
     return CheckAbSpecificMetadata(metadata);
   }
 
-  return 0;
+  return true;
 }
 
-int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd,
+bool SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd,
-                          std::vector<std::string>* cmd) {
+                           std::vector<std::string>* cmd) {
   CHECK(cmd != nullptr);
 
   // For A/B updates we extract the payload properties to a buffer and obtain the RAW payload offset
@@ -240,7 +240,7 @@ int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int
   ZipEntry properties_entry;
   if (FindEntry(zip, property_name, &properties_entry) != 0) {
     LOG(ERROR) << "Failed to find " << AB_OTA_PAYLOAD_PROPERTIES;
-    return INSTALL_CORRUPT;
+    return false;
   }
   uint32_t properties_entry_length = properties_entry.uncompressed_length;
   std::vector<uint8_t> payload_properties(properties_entry_length);
@@ -248,7 +248,7 @@ int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int
       ExtractToMemory(zip, &properties_entry, payload_properties.data(), properties_entry_length);
   if (err != 0) {
     LOG(ERROR) << "Failed to extract " << AB_OTA_PAYLOAD_PROPERTIES << ": " << ErrorCodeString(err);
-    return INSTALL_CORRUPT;
+    return false;
   }
 
   static constexpr const char* AB_OTA_PAYLOAD = "payload.bin";
@@ -256,7 +256,7 @@ int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int
   ZipEntry payload_entry;
   if (FindEntry(zip, payload_name, &payload_entry) != 0) {
     LOG(ERROR) << "Failed to find " << AB_OTA_PAYLOAD;
-    return INSTALL_CORRUPT;
+    return false;
   }
   long payload_offset = payload_entry.offset;
   *cmd = {
@@ -266,11 +266,11 @@ int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int
     "--headers=" + std::string(payload_properties.begin(), payload_properties.end()),
     android::base::StringPrintf("--status_fd=%d", status_fd),
   };
-  return 0;
+  return true;
 }
 
-int SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int retry_count,
+bool SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int retry_count,
-                             int status_fd, std::vector<std::string>* cmd) {
+                              int status_fd, std::vector<std::string>* cmd) {
   CHECK(cmd != nullptr);
 
   // In non-A/B updates we extract the update binary from the package.
@@ -279,7 +279,7 @@ int SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, i
   ZipEntry binary_entry;
   if (FindEntry(zip, binary_name, &binary_entry) != 0) {
     LOG(ERROR) << "Failed to find update binary " << UPDATE_BINARY_NAME;
-    return INSTALL_CORRUPT;
+    return false;
   }
 
   const std::string binary_path = Paths::Get().temporary_update_binary();
@@ -288,13 +288,12 @@ int SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, i
       open(binary_path.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 0755));
   if (fd == -1) {
     PLOG(ERROR) << "Failed to create " << binary_path;
-    return INSTALL_ERROR;
+    return false;
   }
 
-  int32_t error = ExtractEntryToFile(zip, &binary_entry, fd);
+  if (auto error = ExtractEntryToFile(zip, &binary_entry, fd); error != 0) {
-  if (error != 0) {
     LOG(ERROR) << "Failed to extract " << UPDATE_BINARY_NAME << ": " << ErrorCodeString(error);
-    return INSTALL_ERROR;
+    return false;
   }
 
   // When executing the update binary contained in the package, the arguments passed are:
@@ -311,7 +310,7 @@ int SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, i
   if (retry_count > 0) {
     cmd->push_back("retry");
   }
-  return 0;
+  return true;
 }
 
 static void log_max_temperature(int* max_temperature, const std::atomic<bool>& logger_finished) {
@@ -325,9 +324,9 @@ static void log_max_temperature(int* max_temperature, const std::atomic<bool>& l
 }
 
 // If the package contains an update binary, extract it and run it.
-static int try_update_binary(const std::string& package, ZipArchiveHandle zip, bool* wipe_cache,
+static InstallResult TryUpdateBinary(const std::string& package, ZipArchiveHandle zip,
-                             std::vector<std::string>* log_buffer, int retry_count,
+                                     bool* wipe_cache, std::vector<std::string>* log_buffer,
-                             int* max_temperature, RecoveryUI* ui) {
+                                     int retry_count, int* max_temperature, RecoveryUI* ui) {
   std::map<std::string, std::string> metadata;
   if (!ReadMetadataFromPackage(zip, &metadata)) {
     LOG(ERROR) << "Failed to parse metadata in the zip file";
@@ -335,11 +334,10 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b
   }
 
   bool is_ab = android::base::GetBoolProperty("ro.build.ab_update", false);
-  // Verifies against the metadata in the package first.
+  // Verify against the metadata in the package first.
-  if (int check_status = is_ab ? CheckPackageMetadata(metadata, OtaType::AB) : 0;
+  if (is_ab && !CheckPackageMetadata(metadata, OtaType::AB)) {
-      check_status != 0) {
     log_buffer->push_back(android::base::StringPrintf("error: %d", kUpdateBinaryCommandFailure));
-    return check_status;
+    return INSTALL_ERROR;
   }
 
   ReadSourceTargetBuild(metadata, log_buffer);
@@ -386,12 +384,12 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b
   //
 
   std::vector<std::string> args;
-  if (int update_status =
+  if (auto setup_result =
           is_ab ? SetUpAbUpdateCommands(package, zip, pipe_write.get(), &args)
                 : SetUpNonAbUpdateCommands(package, zip, retry_count, pipe_write.get(), &args);
-      update_status != 0) {
+      !setup_result) {
     log_buffer->push_back(android::base::StringPrintf("error: %d", kUpdateBinaryCommandFailure));
-    return update_status;
+    return INSTALL_CORRUPT;
   }
 
   pid_t pid = fork();
@@ -571,9 +569,10 @@ bool verify_package_compatibility(ZipArchiveHandle package_zip) {
   return false;
 }
 
-static int really_install_package(const std::string& path, bool* wipe_cache, bool needs_mount,
+static InstallResult VerifyAndInstallPackage(const std::string& path, bool* wipe_cache,
-                                  std::vector<std::string>* log_buffer, int retry_count,
+                                             bool needs_mount, std::vector<std::string>* log_buffer,
-                                  int* max_temperature, RecoveryUI* ui) {
+                                             int retry_count, int* max_temperature,
+                                             RecoveryUI* ui) {
   ui->SetBackground(RecoveryUI::INSTALLING_UPDATE);
   ui->Print("Finding update package...\n");
   // Give verification half the progress bar...
@@ -624,16 +623,16 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo
     ui->Print("Retry attempt: %d\n", retry_count);
   }
   ui->SetEnableReboot(false);
-  int result =
+  auto result =
-      try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature, ui);
+      TryUpdateBinary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature, ui);
   ui->SetEnableReboot(true);
   ui->Print("\n");
 
   return result;
 }
 
-int install_package(const std::string& path, bool should_wipe_cache, bool needs_mount,
+InstallResult InstallPackage(const std::string& path, bool should_wipe_cache, bool needs_mount,
-                    int retry_count, RecoveryUI* ui) {
+                             int retry_count, RecoveryUI* ui) {
   CHECK(!path.empty());
 
   auto start = std::chrono::system_clock::now();
@@ -641,15 +640,15 @@ int install_package(const std::string& path, bool should_wipe_cache, bool needs_
   int start_temperature = GetMaxValueFromThermalZone();
   int max_temperature = start_temperature;
 
-  int result;
+  InstallResult result;
   std::vector<std::string> log_buffer;
   if (setup_install_mounts() != 0) {
     LOG(ERROR) << "failed to set up expected mounts for install; aborting";
     result = INSTALL_ERROR;
   } else {
     bool updater_wipe_cache = false;
-    result = really_install_package(path, &updater_wipe_cache, needs_mount, &log_buffer,
+    result = VerifyAndInstallPackage(path, &updater_wipe_cache, needs_mount, &log_buffer,
-                                    retry_count, &max_temperature, ui);
+                                     retry_count, &max_temperature, ui);
     should_wipe_cache = should_wipe_cache || updater_wipe_cache;
   }
 

install/wipe_device.cpp

@@ -165,7 +165,7 @@ static bool CheckWipePackage(Package* wipe_package, RecoveryUI* ui) {
     return false;
   }
 
-  return CheckPackageMetadata(metadata, OtaType::BRICK) == 0;
+  return CheckPackageMetadata(metadata, OtaType::BRICK);
 }
 
 bool WipeAbDevice(Device* device, size_t wipe_package_size) {

recovery.cpp

@@ -113,12 +113,12 @@ const char* reason = nullptr;
  * 3. main system reboots into recovery
  * 4. get_args() writes BCB with "boot-recovery" and "--update_package=..."
  *    -- after this, rebooting will attempt to reinstall the update --
- * 5. install_package() attempts to install the update
+ * 5. InstallPackage() attempts to install the update
  *    NOTE: the package install must itself be restartable from any point
  * 6. finish_recovery() erases BCB
  *    -- after this, rebooting will (try to) restart the main system --
  * 7. ** if install failed **
- *    7a. prompt_and_wait() shows an error icon and waits for the user
+ *    7a. PromptAndWait() shows an error icon and waits for the user
  *    7b. the user reboots (pulling the battery, etc) into the main system
  */
 
@@ -312,14 +312,18 @@ static void run_graphics_test() {
   ui->ShowText(true);
 }
 
-// Returns REBOOT, SHUTDOWN, or REBOOT_BOOTLOADER. Returning NO_ACTION means to take the default,
+// Shows the recovery UI and waits for user input. Returns one of the device builtin actions, such
-// which is to reboot or shutdown depending on if the --shutdown_after flag was passed to recovery.
+// as REBOOT, SHUTDOWN, or REBOOT_BOOTLOADER. Returning NO_ACTION means to take the default, which
-static Device::BuiltinAction prompt_and_wait(Device* device, int status) {
+// is to reboot or shutdown depending on if the --shutdown_after flag was passed to recovery.
+static Device::BuiltinAction PromptAndWait(Device* device, InstallResult status) {
   for (;;) {
     finish_recovery();
     switch (status) {
       case INSTALL_SUCCESS:
       case INSTALL_NONE:
+      case INSTALL_SKIPPED:
+      case INSTALL_RETRY:
+      case INSTALL_KEY_INTERRUPTED:
         ui->SetBackground(RecoveryUI::NO_COMMAND);
         break;
 
@@ -327,6 +331,12 @@ static Device::BuiltinAction prompt_and_wait(Device* device, int status) {
       case INSTALL_CORRUPT:
         ui->SetBackground(RecoveryUI::ERROR);
         break;
+
+      case INSTALL_REBOOT:
+        // All the reboots should have been handled prior to entering PromptAndWait() or immediately
+        // after installing a package.
+        LOG(FATAL) << "Invalid status code of INSTALL_REBOOT";
+        break;
     }
     ui->SetProgressType(RecoveryUI::EMPTY);
 
@@ -690,7 +700,7 @@ Device::BuiltinAction start_recovery(Device* device, const std::vector<std::stri
 
   ui->Print("Supported API: %d\n", kRecoveryApiVersion);
 
-  int status = INSTALL_SUCCESS;
+  InstallResult status = INSTALL_SUCCESS;
   // next_action indicates the next target to reboot into upon finishing the install. It could be
   // overridden to a different reboot target per user request.
   Device::BuiltinAction next_action = shutdown_after ? Device::SHUTDOWN : Device::REBOOT;
@@ -720,7 +730,7 @@ Device::BuiltinAction start_recovery(Device* device, const std::vector<std::stri
         set_retry_bootloader_message(retry_count + 1, args);
       }
 
-      status = install_package(update_package, should_wipe_cache, true, retry_count, ui);
+      status = InstallPackage(update_package, should_wipe_cache, true, retry_count, ui);
       if (status != INSTALL_SUCCESS) {
         ui->Print("Installation aborted.\n");
 
@@ -828,7 +838,7 @@ Device::BuiltinAction start_recovery(Device* device, const std::vector<std::stri
   //    for 5s followed by an automatic reboot.
   if (status != INSTALL_REBOOT) {
     if (status == INSTALL_NONE || ui->IsTextVisible()) {
-      Device::BuiltinAction temp = prompt_and_wait(device, status);
+      auto temp = PromptAndWait(device, status);
       if (temp != Device::NO_ACTION) {
         next_action = temp;
       }

tests/unit/install_test.cpp

@@ -205,7 +205,7 @@ TEST(InstallTest, SetUpNonAbUpdateCommands) {
   std::string binary_path = std::string(td.path) + "/update_binary";
   Paths::Get().set_temporary_update_binary(binary_path);
   std::vector<std::string> cmd;
-  ASSERT_EQ(0, SetUpNonAbUpdateCommands(package, zip, 0, status_fd, &cmd));
+  ASSERT_TRUE(SetUpNonAbUpdateCommands(package, zip, 0, status_fd, &cmd));
   ASSERT_EQ(4U, cmd.size());
   ASSERT_EQ(binary_path, cmd[0]);
   ASSERT_EQ("3", cmd[1]);  // RECOVERY_API_VERSION
@@ -217,7 +217,7 @@ TEST(InstallTest, SetUpNonAbUpdateCommands) {
 
   // With non-zero retry count. update_binary will be removed automatically.
   cmd.clear();
-  ASSERT_EQ(0, SetUpNonAbUpdateCommands(package, zip, 2, status_fd, &cmd));
+  ASSERT_TRUE(SetUpNonAbUpdateCommands(package, zip, 2, status_fd, &cmd));
   ASSERT_EQ(5U, cmd.size());
   ASSERT_EQ(binary_path, cmd[0]);
   ASSERT_EQ("3", cmd[1]);  // RECOVERY_API_VERSION
@@ -244,7 +244,7 @@ TEST(InstallTest, SetUpNonAbUpdateCommands_MissingUpdateBinary) {
   TemporaryDir td;
   Paths::Get().set_temporary_update_binary(std::string(td.path) + "/update_binary");
   std::vector<std::string> cmd;
-  ASSERT_EQ(INSTALL_CORRUPT, SetUpNonAbUpdateCommands(package, zip, 0, status_fd, &cmd));
+  ASSERT_FALSE(SetUpNonAbUpdateCommands(package, zip, 0, status_fd, &cmd));
   CloseArchive(zip);
 }
 
@@ -278,12 +278,12 @@ static void VerifyAbUpdateCommands(const std::string& serialno, bool success = t
   std::map<std::string, std::string> metadata;
   ASSERT_TRUE(ReadMetadataFromPackage(zip, &metadata));
   if (success) {
-    ASSERT_EQ(0, CheckPackageMetadata(metadata, OtaType::AB));
+    ASSERT_TRUE(CheckPackageMetadata(metadata, OtaType::AB));
 
     int status_fd = 10;
     std::string package = "/path/to/update.zip";
     std::vector<std::string> cmd;
-    ASSERT_EQ(0, SetUpAbUpdateCommands(package, zip, status_fd, &cmd));
+    ASSERT_TRUE(SetUpAbUpdateCommands(package, zip, status_fd, &cmd));
     ASSERT_EQ(5U, cmd.size());
     ASSERT_EQ("/system/bin/update_engine_sideload", cmd[0]);
     ASSERT_EQ("--payload=file://" + package, cmd[1]);
@@ -291,7 +291,7 @@ static void VerifyAbUpdateCommands(const std::string& serialno, bool success = t
     ASSERT_EQ("--headers=" + properties, cmd[3]);
     ASSERT_EQ("--status_fd=" + std::to_string(status_fd), cmd[4]);
   } else {
-    ASSERT_EQ(INSTALL_ERROR, CheckPackageMetadata(metadata, OtaType::AB));
+    ASSERT_FALSE(CheckPackageMetadata(metadata, OtaType::AB));
   }
   CloseArchive(zip);
 }
@@ -326,7 +326,7 @@ TEST(InstallTest, SetUpAbUpdateCommands_MissingPayloadPropertiesTxt) {
   int status_fd = 10;
   std::string package = "/path/to/update.zip";
   std::vector<std::string> cmd;
-  ASSERT_EQ(INSTALL_CORRUPT, SetUpAbUpdateCommands(package, zip, status_fd, &cmd));
+  ASSERT_FALSE(SetUpAbUpdateCommands(package, zip, status_fd, &cmd));
   CloseArchive(zip);
 }
 
@@ -359,8 +359,8 @@ TEST(InstallTest, SetUpAbUpdateCommands_MultipleSerialnos) {
   VerifyAbUpdateCommands(long_serialno);
 }
 
-static void test_check_package_metadata(const std::string& metadata_string, OtaType ota_type,
+static void TestCheckPackageMetadata(const std::string& metadata_string, OtaType ota_type,
-                                        int exptected_result) {
+                                     bool exptected_result) {
   TemporaryFile temp_file;
   BuildZipArchive(
       {
@@ -388,7 +388,7 @@ TEST(InstallTest, CheckPackageMetadata_ota_type) {
           "post-timestamp=" + std::to_string(std::numeric_limits<int64_t>::max()),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::AB, false);
 
   // Checks if ota-type matches
   metadata = android::base::Join(
@@ -398,9 +398,9 @@ TEST(InstallTest, CheckPackageMetadata_ota_type) {
           "post-timestamp=" + std::to_string(std::numeric_limits<int64_t>::max()),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, 0);
+  TestCheckPackageMetadata(metadata, OtaType::AB, true);
 
-  test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, false);
 }
 
 TEST(InstallTest, CheckPackageMetadata_device_type) {
@@ -410,7 +410,7 @@ TEST(InstallTest, CheckPackageMetadata_device_type) {
           "ota-type=BRICK",
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, false);
 
   // device type mismatches
   metadata = android::base::Join(
@@ -419,7 +419,7 @@ TEST(InstallTest, CheckPackageMetadata_device_type) {
           "pre-device=dummy_device_type",
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, false);
 }
 
 TEST(InstallTest, CheckPackageMetadata_serial_number_smoke) {
@@ -433,7 +433,7 @@ TEST(InstallTest, CheckPackageMetadata_serial_number_smoke) {
           "pre-device=" + device,
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::BRICK, 0);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, true);
 
   // Serial number mismatches
   metadata = android::base::Join(
@@ -443,7 +443,7 @@ TEST(InstallTest, CheckPackageMetadata_serial_number_smoke) {
           "serialno=dummy_serial",
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, false);
 
   std::string serialno = android::base::GetProperty("ro.serialno", "");
   ASSERT_NE("", serialno);
@@ -454,7 +454,7 @@ TEST(InstallTest, CheckPackageMetadata_serial_number_smoke) {
           "serialno=" + serialno,
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::BRICK, 0);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, true);
 }
 
 TEST(InstallTest, CheckPackageMetadata_multiple_serial_number) {
@@ -478,7 +478,7 @@ TEST(InstallTest, CheckPackageMetadata_multiple_serial_number) {
           "serialno=" + android::base::Join(serial_numbers, '|'),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, false);
 
   serial_numbers.emplace_back(serialno);
   std::shuffle(serial_numbers.begin(), serial_numbers.end(), std::default_random_engine());
@@ -489,7 +489,7 @@ TEST(InstallTest, CheckPackageMetadata_multiple_serial_number) {
           "serialno=" + android::base::Join(serial_numbers, '|'),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::BRICK, 0);
+  TestCheckPackageMetadata(metadata, OtaType::BRICK, true);
 }
 
 TEST(InstallTest, CheckPackageMetadata_ab_build_version) {
@@ -507,7 +507,7 @@ TEST(InstallTest, CheckPackageMetadata_ab_build_version) {
           "post-timestamp=" + std::to_string(std::numeric_limits<int64_t>::max()),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, 0);
+  TestCheckPackageMetadata(metadata, OtaType::AB, true);
 
   metadata = android::base::Join(
       std::vector<std::string>{
@@ -517,7 +517,7 @@ TEST(InstallTest, CheckPackageMetadata_ab_build_version) {
           "post-timestamp=" + std::to_string(std::numeric_limits<int64_t>::max()),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::AB, false);
 }
 
 TEST(InstallTest, CheckPackageMetadata_ab_fingerprint) {
@@ -535,7 +535,7 @@ TEST(InstallTest, CheckPackageMetadata_ab_fingerprint) {
           "post-timestamp=" + std::to_string(std::numeric_limits<int64_t>::max()),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, 0);
+  TestCheckPackageMetadata(metadata, OtaType::AB, true);
 
   metadata = android::base::Join(
       std::vector<std::string>{
@@ -545,7 +545,7 @@ TEST(InstallTest, CheckPackageMetadata_ab_fingerprint) {
           "post-timestamp=" + std::to_string(std::numeric_limits<int64_t>::max()),
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::AB, false);
 }
 
 TEST(InstallTest, CheckPackageMetadata_ab_post_timestamp) {
@@ -559,7 +559,7 @@ TEST(InstallTest, CheckPackageMetadata_ab_post_timestamp) {
           "pre-device=" + device,
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::AB, false);
 
   // post timestamp should be larger than the timestamp on device.
   metadata = android::base::Join(
@@ -569,7 +569,7 @@ TEST(InstallTest, CheckPackageMetadata_ab_post_timestamp) {
           "post-timestamp=0",
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::AB, false);
 
   // fingerprint is required for downgrade
   metadata = android::base::Join(
@@ -580,7 +580,7 @@ TEST(InstallTest, CheckPackageMetadata_ab_post_timestamp) {
           "ota-downgrade=yes",
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR);
+  TestCheckPackageMetadata(metadata, OtaType::AB, false);
 
   std::string finger_print = android::base::GetProperty("ro.build.fingerprint", "");
   ASSERT_NE("", finger_print);
@@ -594,5 +594,5 @@ TEST(InstallTest, CheckPackageMetadata_ab_post_timestamp) {
           "ota-downgrade=yes",
       },
       "\n");
-  test_check_package_metadata(metadata, OtaType::AB, 0);
+  TestCheckPackageMetadata(metadata, OtaType::AB, true);
 }