From 09e84938615cacb14f39882d3c4b717b54749ed7 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 31 Aug 2018 11:25:05 -0700 Subject: [PATCH] applypatch: {Load,Save}FileContents return bool values. Bug: 110106408 Test: Run recovery_unit_test and recovery_component_test on marlin. Change-Id: Id72e24dd00eb451565d90cff6e049f4f4b844ea2 --- applypatch/applypatch.cpp | 26 +++++++++++----------- applypatch/include/applypatch/applypatch.h | 9 ++++---- tests/unit/applypatch_test.cpp | 10 ++++----- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index abc67abe..f9383dde 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -49,21 +49,21 @@ using namespace std::string_literals; static bool GenerateTarget(const Partition& target, const FileContents& source_file, const Value& patch, const Value* bonus_data); -int LoadFileContents(const std::string& filename, FileContents* file) { +bool LoadFileContents(const std::string& filename, FileContents* file) { // No longer allow loading contents from eMMC partitions. if (android::base::StartsWith(filename, "EMMC:")) { - return -1; + return false; } std::string data; if (!android::base::ReadFileToString(filename, &data)) { PLOG(ERROR) << "Failed to read \"" << filename << "\""; - return -1; + return false; } file->data = std::vector(data.begin(), data.end()); SHA1(file->data.data(), file->data.size(), file->sha1); - return 0; + return true; } // Reads the contents of a Partition to the given FileContents buffer. @@ -97,7 +97,7 @@ static bool ReadPartitionToBuffer(const Partition& partition, FileContents* out, return false; } - if (LoadFileContents(Paths::Get().cache_temp_source(), out) == 0 && + if (LoadFileContents(Paths::Get().cache_temp_source(), out) && memcmp(out->sha1, expected_sha1, SHA_DIGEST_LENGTH) == 0) { return true; } @@ -106,30 +106,30 @@ static bool ReadPartitionToBuffer(const Partition& partition, FileContents* out, return false; } -int SaveFileContents(const std::string& filename, const FileContents* file) { +bool SaveFileContents(const std::string& filename, const FileContents* file) { android::base::unique_fd fd( open(filename.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_SYNC, S_IRUSR | S_IWUSR)); if (fd == -1) { PLOG(ERROR) << "Failed to open \"" << filename << "\" for write"; - return -1; + return false; } if (!android::base::WriteFully(fd, file->data.data(), file->data.size())) { PLOG(ERROR) << "Failed to write " << file->data.size() << " bytes of data to " << filename; - return -1; + return false; } if (fsync(fd) != 0) { PLOG(ERROR) << "Failed to fsync \"" << filename << "\""; - return -1; + return false; } if (close(fd.release()) != 0) { PLOG(ERROR) << "Failed to close \"" << filename << "\""; - return -1; + return false; } - return 0; + return true; } // Writes a memory buffer to 'target' Partition. @@ -300,7 +300,7 @@ bool FlashPartition(const Partition& partition, const std::string& source_filena } FileContents source_file; - if (LoadFileContents(source_filename, &source_file) != 0) { + if (!LoadFileContents(source_filename, &source_file)) { LOG(ERROR) << "Failed to load source file"; return false; } @@ -355,7 +355,7 @@ static bool GenerateTarget(const Partition& target, const FileContents& source_f LOG(ERROR) << "Not enough free space on /cache"; return false; } - if (SaveFileContents(Paths::Get().cache_temp_source(), &source_file) < 0) { + if (!SaveFileContents(Paths::Get().cache_temp_source(), &source_file)) { LOG(ERROR) << "Failed to back up source file"; return false; } diff --git a/applypatch/include/applypatch/applypatch.h b/applypatch/include/applypatch/applypatch.h index 70eee1c1..6fc6f0fc 100644 --- a/applypatch/include/applypatch/applypatch.h +++ b/applypatch/include/applypatch/applypatch.h @@ -92,12 +92,11 @@ bool CheckPartition(const Partition& target); // function is idempotent. Returns the flashing result. bool FlashPartition(const Partition& target, const std::string& source_filename); -// Reads a file into memory; stores the file contents and associated metadata in *file. Returns 0 -// on success, or -1 on error. -int LoadFileContents(const std::string& filename, FileContents* file); +// Reads a file into memory; stores the file contents and associated metadata in *file. +bool LoadFileContents(const std::string& filename, FileContents* file); -// Saves the given FileContents object to the given filename. Returns 0 on success, or -1 on error. -int SaveFileContents(const std::string& filename, const FileContents* file); +// Saves the given FileContents object to the given filename. +bool SaveFileContents(const std::string& filename, const FileContents* file); // bspatch.cpp diff --git a/tests/unit/applypatch_test.cpp b/tests/unit/applypatch_test.cpp index 4204074e..066f981b 100644 --- a/tests/unit/applypatch_test.cpp +++ b/tests/unit/applypatch_test.cpp @@ -49,13 +49,13 @@ class ApplyPatchTest : public ::testing::Test { void SetUp() override { source_file = from_testdata_base("boot.img"); FileContents boot_fc; - ASSERT_EQ(0, LoadFileContents(source_file, &boot_fc)); + ASSERT_TRUE(LoadFileContents(source_file, &boot_fc)); source_size = boot_fc.data.size(); source_sha1 = print_sha1(boot_fc.sha1); target_file = from_testdata_base("recovery.img"); FileContents recovery_fc; - ASSERT_EQ(0, LoadFileContents(target_file, &recovery_fc)); + ASSERT_TRUE(LoadFileContents(target_file, &recovery_fc)); target_size = recovery_fc.data.size(); target_sha1 = print_sha1(recovery_fc.sha1); @@ -135,11 +135,11 @@ TEST_F(ApplyPatchTest, PatchPartitionCheck_UseBackup_BothCorrupted) { TEST_F(ApplyPatchTest, PatchPartition) { FileContents patch_fc; - ASSERT_EQ(0, LoadFileContents(from_testdata_base("recovery-from-boot.p"), &patch_fc)); + ASSERT_TRUE(LoadFileContents(from_testdata_base("recovery-from-boot.p"), &patch_fc)); Value patch(Value::Type::BLOB, std::string(patch_fc.data.cbegin(), patch_fc.data.cend())); FileContents bonus_fc; - ASSERT_EQ(0, LoadFileContents(from_testdata_base("bonus.file"), &bonus_fc)); + ASSERT_TRUE(LoadFileContents(from_testdata_base("bonus.file"), &bonus_fc)); Value bonus(Value::Type::BLOB, std::string(bonus_fc.data.cbegin(), bonus_fc.data.cend())); ASSERT_TRUE(PatchPartition(target_partition, source_partition, patch, &bonus)); @@ -149,7 +149,7 @@ TEST_F(ApplyPatchTest, PatchPartition) { // everything). TEST_F(ApplyPatchTest, PatchPartitionWithoutBonusFile) { FileContents patch_fc; - ASSERT_EQ(0, LoadFileContents(from_testdata_base("recovery-from-boot-with-bonus.p"), &patch_fc)); + ASSERT_TRUE(LoadFileContents(from_testdata_base("recovery-from-boot-with-bonus.p"), &patch_fc)); Value patch(Value::Type::BLOB, std::string(patch_fc.data.cbegin(), patch_fc.data.cend())); ASSERT_TRUE(PatchPartition(target_partition, source_partition, patch, nullptr));