Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 36c7276c authored by Tao Bao's avatar Tao Bao
Browse files

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
parent 865d1df0

install/include/install/install.h

+4 −4
Original line number Diff line number Diff line
@@ -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.

// Mandatory checks: ota-type, pre-device and serial number(if presents)

// AB OTA specific checks: pre-build version, fingerprint, timestamp.

int CheckPackageMetadata(const std::map<std::string, std::string>& 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<std::string, std::string>& metadata, OtaType ota_type);

install/include/private/setup_commands.h

+4 −4
Original line number Diff line number Diff line
@@ -27,7 +27,7 @@ 
// |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);



// Sets up the commands for an A/B update. Extracts the needed entries from the open zip archive
@@ -35,5 +35,5 @@ int SetUpNonAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, i 
// |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);

install/install.cpp

+31 −33
Original line number Diff line number Diff line
@@ -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,10 +226,10 @@ 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) {

  CHECK(cmd != nullptr);
@@ -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,10 +266,10 @@ 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) {

  CHECK(cmd != nullptr);
@@ -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<bool>& 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<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();

install/wipe_device.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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) {

tests/unit/install_test.cpp

+27 −27
Original line number Diff line number Diff line
@@ -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,

                                        int exptected_result) {

static void TestCheckPackageMetadata(const std::string& metadata_string, OtaType ota_type,

                                     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);

}