From 36c7276cb29c0990933de6da8dbcf7ea2dc2741d Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 30 Apr 2019 00:25:41 -0700 Subject: install: Return bool for a few check functions. The results from these functions have boolean semantics. They're returning `int` prior to this CL, with some of them mixing 0 and InstallResult. Note that SetUpNonAbUpdateCommands() was returning INSTALL_CORRUPT / INSTALL_ERROR / 0 prior to this change, but all the callers handle INSTALL_CORRUPT and INSTALL_ERROR the same way. This CL changes them to return bool instead. Test: `mmma -j bootable/recovery` Test: TreeHugger Test: Sideload on taimen. Change-Id: Ic1b5dbf79aaca68b53ab8ea2c8ba3d19f988c571 --- install/include/install/install.h | 8 ++-- install/include/private/setup_commands.h | 8 ++-- install/install.cpp | 64 ++++++++++++++++---------------- install/wipe_device.cpp | 2 +- 4 files changed, 40 insertions(+), 42 deletions(-) (limited to 'install') diff --git a/install/include/install/install.h b/install/include/install/install.h index d90c20f3a..cc595f6b0 100644 --- a/install/include/install/install.h +++ b/install/include/install/install.h @@ -62,7 +62,7 @@ bool ReadMetadataFromPackage(ZipArchiveHandle zip, std::map& metadata, OtaType ota_type); +// Checks if the metadata in the OTA package has expected values. Mandatory checks: ota-type, +// pre-device and serial number (if presents). A/B OTA specific checks: pre-build version, +// fingerprint, timestamp. +bool CheckPackageMetadata(const std::map& metadata, OtaType ota_type); diff --git a/install/include/private/setup_commands.h b/install/include/private/setup_commands.h index 7fdc741d6..dcff76112 100644 --- a/install/include/private/setup_commands.h +++ b/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, - int status_fd, std::vector* cmd); +bool SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int retry_count, + int status_fd, std::vector* 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, - std::vector* cmd); +bool SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd, + std::vector* cmd); diff --git a/install/install.cpp b/install/install.cpp index e2d470096..3f4df13e1 100644 --- a/install/install.cpp +++ b/install/install.cpp @@ -139,14 +139,14 @@ static void ReadSourceTargetBuild(const std::map& 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& metadata) { +static bool CheckAbSpecificMetadata(const std::map& 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& 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& 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& metadata, OtaType ota_type) { +bool CheckPackageMetadata(const std::map& 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& 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& metadata, Ota return CheckAbSpecificMetadata(metadata); } - return 0; + return true; } -int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd, - std::vector* cmd) { +bool SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd, + std::vector* 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 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, - int status_fd, std::vector* cmd) { +bool SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int retry_count, + int status_fd, std::vector* 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 (error != 0) { + if (auto error = ExtractEntryToFile(zip, &binary_entry, fd); 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& logger_finished) { @@ -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. - if (int check_status = is_ab ? CheckPackageMetadata(metadata, OtaType::AB) : 0; - check_status != 0) { + // Verify against the metadata in the package first. + if (is_ab && !CheckPackageMetadata(metadata, OtaType::AB)) { 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 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(); diff --git a/install/wipe_device.cpp b/install/wipe_device.cpp index 72b96f7b4..5a9b512c1 100644 --- a/install/wipe_device.cpp +++ b/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) { -- cgit v1.2.3 From adc99efd1cfd94283c6a1c5c9e9a4ce4af2c5daf Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 29 Apr 2019 23:48:02 -0700 Subject: install: Install functions return InstallResult. Test: `atest recovery_unit_test recovery_component_test` Test: Sideload a package on taimen. Change-Id: I2d42f55a89931ee495ea5c5d9e6b5ee1058e8e52 --- install/adb_install.cpp | 10 +++++----- install/fuse_sdcard_install.cpp | 9 ++++----- install/include/install/adb_install.h | 11 ++++++----- install/include/install/fuse_sdcard_install.h | 3 ++- install/include/install/install.h | 4 ++-- install/install.cpp | 27 ++++++++++++++------------- 6 files changed, 33 insertions(+), 31 deletions(-) (limited to 'install') diff --git a/install/adb_install.cpp b/install/adb_install.cpp index 4dd1f1b09..2de1075d2 100644 --- a/install/adb_install.cpp +++ b/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 command_map{ { MinadbdCommand::kInstall, std::bind(&AdbInstallPackageHandler, ui, &install_result) }, { MinadbdCommand::kRebootAndroid, std::bind(&AdbRebootHandler, MinadbdCommand::kRebootAndroid, diff --git a/install/fuse_sdcard_install.cpp b/install/fuse_sdcard_install.cpp index 1aa8768e7..a5caa6e75 100644 --- a/install/fuse_sdcard_install.cpp +++ b/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 - // process is ready. - int result = INSTALL_ERROR; + // FUSE_SIDELOAD_HOST_PATHNAME will start to exist once the fuse in child process is ready. + InstallResult 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; } diff --git a/install/include/install/adb_install.h b/install/include/install/adb_install.h index 3a0a81747..880022361 100644 --- a/install/include/install/adb_install.h +++ b/install/include/install/adb_install.h @@ -16,9 +16,10 @@ #pragma once -#include +#include "install/install.h" +#include "recovery_ui/device.h" -// Applies a package via `adb sideload` or `adb rescue`. Returns the install result (in `enum -// InstallResult`). When a reboot has been requested, INSTALL_REBOOT will be the return value, with -// the reboot target set in reboot_action. -int ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action); +// Applies a package via `adb sideload` or `adb rescue`. Returns the install result. When a reboot +// has been requested, INSTALL_REBOOT will be the return value, with the reboot target set in +// reboot_action. +InstallResult ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action); diff --git a/install/include/install/fuse_sdcard_install.h b/install/include/install/fuse_sdcard_install.h index d9214ca3b..e5bb01f09 100644 --- a/install/include/install/fuse_sdcard_install.h +++ b/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); diff --git a/install/include/install/install.h b/install/include/install/install.h index cc595f6b0..44a5cde91 100644 --- a/install/include/install/install.h +++ b/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, - int retry_count, RecoveryUI* ui); +InstallResult InstallPackage(const std::string& package, bool should_wipe_cache, bool needs_mount, + int retry_count, RecoveryUI* ui); // Verifies the package by ota keys. Returns true if the package is verified successfully, // otherwise returns false. diff --git a/install/install.cpp b/install/install.cpp index 3f4df13e1..5d514fa23 100644 --- a/install/install.cpp +++ b/install/install.cpp @@ -324,9 +324,9 @@ static void log_max_temperature(int* max_temperature, const std::atomic& 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, - std::vector* log_buffer, int retry_count, - int* max_temperature, RecoveryUI* ui) { +static InstallResult TryUpdateBinary(const std::string& package, ZipArchiveHandle zip, + bool* wipe_cache, std::vector* log_buffer, + int retry_count, int* max_temperature, RecoveryUI* ui) { std::map metadata; if (!ReadMetadataFromPackage(zip, &metadata)) { LOG(ERROR) << "Failed to parse metadata in the zip file"; @@ -569,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, - std::vector* log_buffer, int retry_count, - int* max_temperature, RecoveryUI* ui) { +static InstallResult VerifyAndInstallPackage(const std::string& path, bool* wipe_cache, + bool needs_mount, std::vector* log_buffer, + 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... @@ -622,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 = - try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature, ui); + auto result = + 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, - int retry_count, RecoveryUI* ui) { +InstallResult InstallPackage(const std::string& path, bool should_wipe_cache, bool needs_mount, + int retry_count, RecoveryUI* ui) { CHECK(!path.empty()); auto start = std::chrono::system_clock::now(); @@ -639,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 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, - retry_count, &max_temperature, ui); + result = VerifyAndInstallPackage(path, &updater_wipe_cache, needs_mount, &log_buffer, + retry_count, &max_temperature, ui); should_wipe_cache = should_wipe_cache || updater_wipe_cache; } -- cgit v1.2.3