From bd0fb2309b45bab61eb2444f0589a6a23fc0f88f Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 11 Jul 2018 23:33:57 -0700 Subject: applypatch: Switch freecache.cpp to libbase logging. Test: mmma -j bootable/recovery Test: Run recovery_unit_test on marlin. Change-Id: Iaa326cd26211d527071cc64a928e59a96271b111 --- applypatch/freecache.cpp | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/applypatch/freecache.cpp b/applypatch/freecache.cpp index dbd4b72b1..5a08a63ed 100644 --- a/applypatch/freecache.cpp +++ b/applypatch/freecache.cpp @@ -14,16 +14,12 @@ * limitations under the License. */ -#include #include #include -#include -#include #include #include #include #include -#include #include #include @@ -33,6 +29,7 @@ #include #include +#include #include #include #include @@ -43,7 +40,7 @@ static int EliminateOpenFiles(const std::string& dirname, std::set* files) { std::unique_ptr d(opendir("/proc"), closedir); if (!d) { - printf("error opening /proc: %s\n", strerror(errno)); + PLOG(ERROR) << "Failed to open /proc"; return -1; } struct dirent* de; @@ -57,7 +54,7 @@ static int EliminateOpenFiles(const std::string& dirname, std::set* struct dirent* fdde; std::unique_ptr fdd(opendir(path.c_str()), closedir); if (!fdd) { - printf("error opening %s: %s\n", path.c_str(), strerror(errno)); + PLOG(ERROR) << "Failed to open " << path; continue; } while ((fdde = readdir(fdd.get())) != 0) { @@ -69,7 +66,7 @@ static int EliminateOpenFiles(const std::string& dirname, std::set* link[count] = '\0'; if (android::base::StartsWith(link, dirname)) { if (files->erase(link) > 0) { - printf("%s is open by %s\n", link, de->d_name); + LOG(INFO) << link << " is open by " << de->d_name; } } } @@ -80,15 +77,14 @@ static int EliminateOpenFiles(const std::string& dirname, std::set* static std::vector FindExpendableFiles( const std::string& dirname, const std::function& name_filter) { - std::set files; - std::unique_ptr d(opendir(dirname.c_str()), closedir); if (!d) { - printf("error opening %s: %s\n", dirname.c_str(), strerror(errno)); + PLOG(ERROR) << "Failed to open " << dirname; return {}; } // Look for regular files in the directory (not in any subdirectories). + std::set files; struct dirent* de; while ((de = readdir(d.get())) != 0) { std::string path = dirname + "/" + de->d_name; @@ -110,7 +106,7 @@ static std::vector FindExpendableFiles( } } - printf("%zu regular files in deletable directory\n", files.size()); + LOG(INFO) << files.size() << " regular files in deletable directory"; if (EliminateOpenFiles(dirname, &files) < 0) { return {}; } @@ -136,13 +132,13 @@ static unsigned int GetLogIndex(const std::string& log_name) { int MakeFreeSpaceOnCache(size_t bytes_needed) { #ifndef __ANDROID__ - // TODO (xunchang) implement a heuristic cache size check during host simulation. - printf("Skip making (%zu) bytes free space on /cache; program is running on host\n", - bytes_needed); + // TODO(xunchang): Implement a heuristic cache size check during host simulation. + LOG(WARNING) << "Skipped making (" << bytes_needed + << ") bytes free space on /cache; program is running on host"; return 0; #endif - std::vector dirs = { "/cache", Paths::Get().cache_log_directory() }; + std::vector dirs{ "/cache", Paths::Get().cache_log_directory() }; for (const auto& dirname : dirs) { if (RemoveFilesInDirectory(bytes_needed, dirname, FreeSpaceForFile)) { return 0; @@ -155,17 +151,17 @@ int MakeFreeSpaceOnCache(size_t bytes_needed) { bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname, const std::function& space_checker) { struct stat st; - if (stat(dirname.c_str(), &st) != 0) { - error(0, errno, "Unable to free space on %s", dirname.c_str()); + if (stat(dirname.c_str(), &st) == -1) { + PLOG(ERROR) << "Failed to stat " << dirname; return false; } if (!S_ISDIR(st.st_mode)) { - printf("%s is not a directory\n", dirname.c_str()); + LOG(ERROR) << dirname << " is not a directory"; return false; } size_t free_now = space_checker(dirname); - printf("%zu bytes free on %s (%zu needed)\n", free_now, dirname.c_str(), bytes_needed); + LOG(INFO) << free_now << " bytes free on " << dirname << " (" << bytes_needed << " needed)"; if (free_now >= bytes_needed) { return true; @@ -200,12 +196,12 @@ bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname, for (const auto& file : files) { if (unlink(file.c_str()) == -1) { - error(0, errno, "Failed to delete %s", file.c_str()); + PLOG(ERROR) << "Failed to delete " << file; continue; } free_now = space_checker(dirname); - printf("deleted %s; now %zu bytes free\n", file.c_str(), free_now); + LOG(INFO) << "Deleted " << file << "; now " << free_now << " bytes free"; if (free_now >= bytes_needed) { return true; } -- cgit v1.2.3 From 49750f15ed2db55247cc03170fec1eed617d9454 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 11 Jul 2018 16:32:10 -0700 Subject: applypatch: Fix the return type of FreeSpaceForFile(). Prior to this CL, FreeSpaceForFile() was returning `size_t`, which may overflow on ILP32 when called on a partition with 4GiB+ free space. Additionally, it was returning static_cast(-1) on error, but the caller in freecache.cpp didn't check for that. This CL changes its return type to `int64_t`, and moves the function into freecache.cpp since there's no external caller. Test: Run recovery_unit_test on marlin. Test: Code search shows no external user of FreeSpaceForFile(). Change-Id: I00f501a057726e1f1ab69f367c46c77b30f2d774 --- applypatch/applypatch.cpp | 10 ---------- applypatch/freecache.cpp | 32 ++++++++++++++++++++++++++++-- applypatch/include/applypatch/applypatch.h | 10 +++------- tests/unit/applypatch_test.cpp | 24 ++++++++++++++++------ 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index eb0a2a7b5..13e4b1ae0 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include @@ -421,15 +420,6 @@ static size_t FileSink(const unsigned char* data, size_t len, int fd) { return done; } -size_t FreeSpaceForFile(const std::string& filename) { - struct statfs sf; - if (statfs(filename.c_str(), &sf) != 0) { - PLOG(ERROR) << "Failed to statfs " << filename; - return -1; - } - return sf.f_bsize * sf.f_bavail; -} - int CacheSizeCheck(size_t bytes) { if (MakeFreeSpaceOnCache(bytes) < 0) { LOG(ERROR) << "Failed to make " << bytes << " bytes available on /cache"; diff --git a/applypatch/freecache.cpp b/applypatch/freecache.cpp index 5a08a63ed..4989b7496 100644 --- a/applypatch/freecache.cpp +++ b/applypatch/freecache.cpp @@ -16,10 +16,12 @@ #include #include +#include #include #include #include #include +#include #include #include @@ -130,6 +132,24 @@ static unsigned int GetLogIndex(const std::string& log_name) { return std::numeric_limits::max(); } +// Returns the amount of free space (in bytes) on the filesystem containing filename, or -1 on +// error. +static int64_t FreeSpaceForFile(const std::string& filename) { + struct statfs sf; + if (statfs(filename.c_str(), &sf) == -1) { + PLOG(ERROR) << "Failed to statfs " << filename; + return -1; + } + + int64_t free_space = static_cast(sf.f_bsize) * sf.f_bavail; + if (sf.f_bsize == 0 || free_space / sf.f_bsize != sf.f_bavail) { + LOG(ERROR) << "Invalid block size or overflow (sf.f_bsize " << sf.f_bsize << ", sf.f_bavail " + << sf.f_bavail << ")"; + return -1; + } + return free_space; +} + int MakeFreeSpaceOnCache(size_t bytes_needed) { #ifndef __ANDROID__ // TODO(xunchang): Implement a heuristic cache size check during host simulation. @@ -149,7 +169,7 @@ int MakeFreeSpaceOnCache(size_t bytes_needed) { } bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname, - const std::function& space_checker) { + const std::function& space_checker) { struct stat st; if (stat(dirname.c_str(), &st) == -1) { PLOG(ERROR) << "Failed to stat " << dirname; @@ -160,7 +180,11 @@ bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname, return false; } - size_t free_now = space_checker(dirname); + int64_t free_now = space_checker(dirname); + if (free_now == -1) { + LOG(ERROR) << "Failed to check free space for " << dirname; + return false; + } LOG(INFO) << free_now << " bytes free on " << dirname << " (" << bytes_needed << " needed)"; if (free_now >= bytes_needed) { @@ -201,6 +225,10 @@ bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname, } free_now = space_checker(dirname); + if (free_now == -1) { + LOG(ERROR) << "Failed to check free space for " << dirname; + return false; + } LOG(INFO) << "Deleted " << file << "; now " << free_now << " bytes free"; if (free_now >= bytes_needed) { return true; diff --git a/applypatch/include/applypatch/applypatch.h b/applypatch/include/applypatch/applypatch.h index 92db59c3a..88659b86a 100644 --- a/applypatch/include/applypatch/applypatch.h +++ b/applypatch/include/applypatch/applypatch.h @@ -40,10 +40,6 @@ using SinkFn = std::function; int ShowLicenses(); -// Returns the amount of free space (in bytes) on the filesystem containing filename, or -1 on -// error. filename must exist. -size_t FreeSpaceForFile(const std::string& filename); - // Checks whether /cache partition has at least 'bytes'-byte free space. Returns 0 on having // sufficient space. int CacheSizeCheck(size_t bytes); @@ -119,8 +115,8 @@ int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const Value& int MakeFreeSpaceOnCache(size_t bytes_needed); -// Removes the files in |dirname| until we have at least |bytes_needed| bytes of free space on -// the partition. The size of the free space is returned by calling |space_checker|. +// Removes the files in |dirname| until we have at least |bytes_needed| bytes of free space on the +// partition. |space_checker| should return the size of the free space, or -1 on error. bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname, - const std::function& space_checker); + const std::function& space_checker); #endif diff --git a/tests/unit/applypatch_test.cpp b/tests/unit/applypatch_test.cpp index 5cc03bc7b..4b757feab 100644 --- a/tests/unit/applypatch_test.cpp +++ b/tests/unit/applypatch_test.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -29,6 +30,7 @@ #include #include +#include #include #include #include @@ -176,7 +178,7 @@ class FreeCacheTest : public ::testing::Test { // A mock method to calculate the free space. It assumes the partition has a total size of 40960 // bytes and all files are 4096 bytes in size. - size_t MockFreeSpaceChecker(const std::string& dirname) { + int64_t MockFreeSpaceChecker(const std::string& dirname) { std::vector files = FindFilesInDir(dirname); return PARTITION_SIZE - 4096 * files.size(); } @@ -191,7 +193,7 @@ TEST_F(FreeCacheTest, FreeCacheSmoke) { ASSERT_EQ(files, FindFilesInDir(mock_cache.path)); ASSERT_EQ(4096 * 7, MockFreeSpaceChecker(mock_cache.path)); - ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_cache.path, [&](const std::string& dir) { + ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_cache.path, [this](const std::string& dir) { return this->MockFreeSpaceChecker(dir); })); @@ -199,6 +201,16 @@ TEST_F(FreeCacheTest, FreeCacheSmoke) { ASSERT_EQ(4096 * 9, MockFreeSpaceChecker(mock_cache.path)); } +TEST_F(FreeCacheTest, FreeCacheFreeSpaceCheckerError) { + std::vector files{ "file1", "file2", "file3" }; + AddFilesToDir(mock_cache.path, files); + ASSERT_EQ(files, FindFilesInDir(mock_cache.path)); + ASSERT_EQ(4096 * 7, MockFreeSpaceChecker(mock_cache.path)); + + ASSERT_FALSE( + RemoveFilesInDirectory(4096 * 9, mock_cache.path, [](const std::string&) { return -1; })); +} + TEST_F(FreeCacheTest, FreeCacheOpenFile) { std::vector files = { "file1", "file2" }; AddFilesToDir(mock_cache.path, files); @@ -209,7 +221,7 @@ TEST_F(FreeCacheTest, FreeCacheOpenFile) { android::base::unique_fd fd(open(file1_path.c_str(), O_RDONLY)); // file1 can't be deleted as it's opened by us. - ASSERT_FALSE(RemoveFilesInDirectory(4096 * 10, mock_cache.path, [&](const std::string& dir) { + ASSERT_FALSE(RemoveFilesInDirectory(4096 * 10, mock_cache.path, [this](const std::string& dir) { return this->MockFreeSpaceChecker(dir); })); @@ -222,7 +234,7 @@ TEST_F(FreeCacheTest, FreeCacheLogsSmoke) { AddFilesToDir(mock_log_dir.path, log_files); ASSERT_EQ(4096 * 5, MockFreeSpaceChecker(mock_log_dir.path)); - ASSERT_TRUE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, [&](const std::string& dir) { + ASSERT_TRUE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, [this](const std::string& dir) { return this->MockFreeSpaceChecker(dir); })); @@ -238,7 +250,7 @@ TEST_F(FreeCacheTest, FreeCacheLogsStringComparison) { AddFilesToDir(mock_log_dir.path, log_files); ASSERT_EQ(4096 * 6, MockFreeSpaceChecker(mock_log_dir.path)); - ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_log_dir.path, [&](const std::string& dir) { + ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_log_dir.path, [this](const std::string& dir) { return this->MockFreeSpaceChecker(dir); })); @@ -255,7 +267,7 @@ TEST_F(FreeCacheTest, FreeCacheLogsOtherFiles) { AddFilesToDir(mock_log_dir.path, log_files); ASSERT_EQ(4096 * 5, MockFreeSpaceChecker(mock_log_dir.path)); - ASSERT_FALSE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, [&](const std::string& dir) { + ASSERT_FALSE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, [this](const std::string& dir) { return this->MockFreeSpaceChecker(dir); })); -- cgit v1.2.3 From b1c5b62557a7bb3dbae2c5614f3e9bd053c383a3 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 12 Jul 2018 11:49:05 -0700 Subject: tests: Clean up the temporary dirs post-run. TemporaryDir only deletes empty dirs (not done by its dtor because it tries to keep the temporary files available on error exit). Also change FreeCacheTest::MockFreeSpaceChecker to be static. Test: Run recovery_unit_test on marlin. Check /data/local/tmp post-run. Change-Id: I1bd54eb840e3094b4f22ee84c059eec2998773bf --- tests/unit/applypatch_test.cpp | 38 +++++++++++++++++++------------------- tests/unit/screen_ui_test.cpp | 5 +++-- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/tests/unit/applypatch_test.cpp b/tests/unit/applypatch_test.cpp index 4b757feab..0d4123b5e 100644 --- a/tests/unit/applypatch_test.cpp +++ b/tests/unit/applypatch_test.cpp @@ -164,27 +164,37 @@ class FreeCacheTest : public ::testing::Test { return file_list; } - static void AddFilesToDir(const std::string& dir, const std::vector& files) { + void AddFilesToDir(const std::string& dir, const std::vector& files) { std::string zeros(4096, 0); for (const auto& file : files) { - std::string path = dir + "/" + file; - ASSERT_TRUE(android::base::WriteStringToFile(zeros, path)); + temporary_files_.push_back(dir + "/" + file); + ASSERT_TRUE(android::base::WriteStringToFile(zeros, temporary_files_.back())); } } void SetUp() override { Paths::Get().set_cache_log_directory(mock_log_dir.path); + temporary_files_.clear(); + } + + void TearDown() override { + for (const auto& file : temporary_files_) { + ASSERT_TRUE(android::base::RemoveFileIfExists(file)); + } } // A mock method to calculate the free space. It assumes the partition has a total size of 40960 // bytes and all files are 4096 bytes in size. - int64_t MockFreeSpaceChecker(const std::string& dirname) { + static size_t MockFreeSpaceChecker(const std::string& dirname) { std::vector files = FindFilesInDir(dirname); return PARTITION_SIZE - 4096 * files.size(); } TemporaryDir mock_cache; TemporaryDir mock_log_dir; + + private: + std::vector temporary_files_; }; TEST_F(FreeCacheTest, FreeCacheSmoke) { @@ -193,9 +203,7 @@ TEST_F(FreeCacheTest, FreeCacheSmoke) { ASSERT_EQ(files, FindFilesInDir(mock_cache.path)); ASSERT_EQ(4096 * 7, MockFreeSpaceChecker(mock_cache.path)); - ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_cache.path, [this](const std::string& dir) { - return this->MockFreeSpaceChecker(dir); - })); + ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_cache.path, MockFreeSpaceChecker)); ASSERT_EQ(std::vector{ "file3" }, FindFilesInDir(mock_cache.path)); ASSERT_EQ(4096 * 9, MockFreeSpaceChecker(mock_cache.path)); @@ -221,9 +229,7 @@ TEST_F(FreeCacheTest, FreeCacheOpenFile) { android::base::unique_fd fd(open(file1_path.c_str(), O_RDONLY)); // file1 can't be deleted as it's opened by us. - ASSERT_FALSE(RemoveFilesInDirectory(4096 * 10, mock_cache.path, [this](const std::string& dir) { - return this->MockFreeSpaceChecker(dir); - })); + ASSERT_FALSE(RemoveFilesInDirectory(4096 * 10, mock_cache.path, MockFreeSpaceChecker)); ASSERT_EQ(std::vector{ "file1" }, FindFilesInDir(mock_cache.path)); } @@ -234,9 +240,7 @@ TEST_F(FreeCacheTest, FreeCacheLogsSmoke) { AddFilesToDir(mock_log_dir.path, log_files); ASSERT_EQ(4096 * 5, MockFreeSpaceChecker(mock_log_dir.path)); - ASSERT_TRUE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, [this](const std::string& dir) { - return this->MockFreeSpaceChecker(dir); - })); + ASSERT_TRUE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, MockFreeSpaceChecker)); // Logs with a higher index will be deleted first std::vector expected = { "last_log", "last_log.1" }; @@ -250,9 +254,7 @@ TEST_F(FreeCacheTest, FreeCacheLogsStringComparison) { AddFilesToDir(mock_log_dir.path, log_files); ASSERT_EQ(4096 * 6, MockFreeSpaceChecker(mock_log_dir.path)); - ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_log_dir.path, [this](const std::string& dir) { - return this->MockFreeSpaceChecker(dir); - })); + ASSERT_TRUE(RemoveFilesInDirectory(4096 * 9, mock_log_dir.path, MockFreeSpaceChecker)); // Logs with incorrect format will be deleted first; and the last_kmsg with the same index is // deleted before last_log. @@ -267,9 +269,7 @@ TEST_F(FreeCacheTest, FreeCacheLogsOtherFiles) { AddFilesToDir(mock_log_dir.path, log_files); ASSERT_EQ(4096 * 5, MockFreeSpaceChecker(mock_log_dir.path)); - ASSERT_FALSE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, [this](const std::string& dir) { - return this->MockFreeSpaceChecker(dir); - })); + ASSERT_FALSE(RemoveFilesInDirectory(4096 * 8, mock_log_dir.path, MockFreeSpaceChecker)); // Non log files in /cache/recovery won't be deleted. std::vector expected = { "block.map", "command", "last_install" }; diff --git a/tests/unit/screen_ui_test.cpp b/tests/unit/screen_ui_test.cpp index 2f4b7b09b..4c0a868f0 100644 --- a/tests/unit/screen_ui_test.cpp +++ b/tests/unit/screen_ui_test.cpp @@ -446,8 +446,9 @@ TEST_F(ScreenRecoveryUITest, LoadAnimation_MissingAnimation) { RETURN_IF_NO_GRAPHICS; ASSERT_TRUE(ui_->Init(kTestLocale)); - TemporaryDir resource_dir; - Paths::Get().set_resource_dir(resource_dir.path); + // We need a dir that doesn't contain any animation. However, using TemporaryDir will give + // leftovers since this is a death test where TemporaryDir::~TemporaryDir() won't be called. + Paths::Get().set_resource_dir("/proc/self"); ::testing::FLAGS_gtest_death_test_style = "threadsafe"; ASSERT_EXIT(ui_->RunLoadAnimation(), ::testing::KilledBySignal(SIGABRT), ""); -- cgit v1.2.3 From 5ee25666cc819e9ebc9b72c7a44c4bc9bab9e4e3 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 11 Jul 2018 15:55:32 -0700 Subject: applypatch: Consolidate CacheSizeCheck() and MakeFreeSpaceOnCache(). They are doing exactly the same thing, except for the slightly different error return value (1 vs -1). int CacheSizeCheck(size_t bytes); int MakeFreeSpaceOnCache(size_t bytes_needed); This CL consolidates the two functions and uses bool as its return type. // Checks whether /cache partition has at least 'bytes'-byte free space. Returns true immediately // if so. Otherwise, it will try to free some space by removing older logs, checks again and // returns the checking result. bool CheckAndFreeSpaceOnCache(size_t bytes); Test: Run recovery_unit_test and recovery_component_test on marlin. Change-Id: I94a96934d2b18713f8f39ad5aa96a02c98d87963 --- applypatch/applypatch.cpp | 12 ++---------- applypatch/freecache.cpp | 12 ++++++------ applypatch/include/applypatch/applypatch.h | 9 ++++----- updater/blockimg.cpp | 6 +++--- updater/install.cpp | 2 +- 5 files changed, 16 insertions(+), 25 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index 13e4b1ae0..12e139930 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -420,14 +420,6 @@ static size_t FileSink(const unsigned char* data, size_t len, int fd) { return done; } -int CacheSizeCheck(size_t bytes) { - if (MakeFreeSpaceOnCache(bytes) < 0) { - LOG(ERROR) << "Failed to make " << bytes << " bytes available on /cache"; - return 1; - } - return 0; -} - int applypatch(const char* source_filename, const char* target_filename, const char* target_sha1_str, size_t /* target_size */, const std::vector& patch_sha1s, @@ -562,8 +554,8 @@ static int GenerateTarget(const FileContents& source_file, const std::unique_ptr CHECK(android::base::StartsWith(target_filename, "EMMC:")); - // We still write the original source to cache, in case the partition write is interrupted. - if (MakeFreeSpaceOnCache(source_file.data.size()) < 0) { + // We write the original source to cache, in case the partition write is interrupted. + if (!CheckAndFreeSpaceOnCache(source_file.data.size())) { LOG(ERROR) << "Not enough free space on /cache"; return 1; } diff --git a/applypatch/freecache.cpp b/applypatch/freecache.cpp index 4989b7496..e4878655e 100644 --- a/applypatch/freecache.cpp +++ b/applypatch/freecache.cpp @@ -150,22 +150,22 @@ static int64_t FreeSpaceForFile(const std::string& filename) { return free_space; } -int MakeFreeSpaceOnCache(size_t bytes_needed) { +bool CheckAndFreeSpaceOnCache(size_t bytes) { #ifndef __ANDROID__ // TODO(xunchang): Implement a heuristic cache size check during host simulation. - LOG(WARNING) << "Skipped making (" << bytes_needed + LOG(WARNING) << "Skipped making (" << bytes << ") bytes free space on /cache; program is running on host"; - return 0; + return true; #endif std::vector dirs{ "/cache", Paths::Get().cache_log_directory() }; for (const auto& dirname : dirs) { - if (RemoveFilesInDirectory(bytes_needed, dirname, FreeSpaceForFile)) { - return 0; + if (RemoveFilesInDirectory(bytes, dirname, FreeSpaceForFile)) { + return true; } } - return -1; + return false; } bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname, diff --git a/applypatch/include/applypatch/applypatch.h b/applypatch/include/applypatch/applypatch.h index 88659b86a..28dba7e68 100644 --- a/applypatch/include/applypatch/applypatch.h +++ b/applypatch/include/applypatch/applypatch.h @@ -40,10 +40,6 @@ using SinkFn = std::function; int ShowLicenses(); -// Checks whether /cache partition has at least 'bytes'-byte free space. Returns 0 on having -// sufficient space. -int CacheSizeCheck(size_t bytes); - // Parses a given string of 40 hex digits into 20-byte array 'digest'. 'str' may contain only the // digest or be of the form ":". Returns 0 on success, or -1 on any error. int ParseSha1(const std::string& str, uint8_t* digest); @@ -113,7 +109,10 @@ int ApplyImagePatch(const unsigned char* old_data, size_t old_size, const Value& // freecache.cpp -int MakeFreeSpaceOnCache(size_t bytes_needed); +// Checks whether /cache partition has at least 'bytes'-byte free space. Returns true immediately +// if so. Otherwise, it will try to free some space by removing older logs, checks again and +// returns the checking result. +bool CheckAndFreeSpaceOnCache(size_t bytes); // Removes the files in |dirname| until we have at least |bytes_needed| bytes of free space on the // partition. |space_checker| should return the size of the free space, or -1 on error. diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 6a6236b1b..2a2ab19a3 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -827,7 +827,7 @@ static int WriteStash(const std::string& base, const std::string& id, int blocks return -1; } - if (checkspace && CacheSizeCheck(blocks * BLOCKSIZE) != 0) { + if (checkspace && !CheckAndFreeSpaceOnCache(blocks * BLOCKSIZE)) { LOG(ERROR) << "not enough space to write stash"; return -1; } @@ -919,7 +919,7 @@ static int CreateStash(State* state, size_t maxblocks, const std::string& base) return -1; } - if (CacheSizeCheck(max_stash_size) != 0) { + if (!CheckAndFreeSpaceOnCache(max_stash_size)) { ErrorAbort(state, kStashCreationFailure, "not enough space for stash (%zu needed)", max_stash_size); return -1; @@ -951,7 +951,7 @@ static int CreateStash(State* state, size_t maxblocks, const std::string& base) if (max_stash_size > existing) { size_t needed = max_stash_size - existing; - if (CacheSizeCheck(needed) != 0) { + if (!CheckAndFreeSpaceOnCache(needed)) { ErrorAbort(state, kStashCreationFailure, "not enough space for stash (%zu more needed)", needed); return -1; diff --git a/updater/install.cpp b/updater/install.cpp index d0be955a7..088d24b31 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -688,7 +688,7 @@ Value* ApplyPatchSpaceFn(const char* name, State* state, } // Skip the cache size check if the update is a retry. - if (state->is_retry || CacheSizeCheck(bytes) == 0) { + if (state->is_retry || CheckAndFreeSpaceOnCache(bytes)) { return StringValue("t"); } return StringValue(""); -- cgit v1.2.3