Merge "applypatch: Fix the return type of FreeSpaceForFile()." am: 90c9e4f55c am: c9f676bc64
am: 22c5aed89e
Change-Id: Ibdf05b7f50a5a04b5a6994fded638d0d39414a5d
diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp
index eb0a2a7..13e4b1a 100644
--- a/applypatch/applypatch.cpp
+++ b/applypatch/applypatch.cpp
@@ -23,7 +23,6 @@
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
-#include <sys/statfs.h>
#include <sys/types.h>
#include <unistd.h>
@@ -421,15 +420,6 @@
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 5a08a63..4989b74 100644
--- a/applypatch/freecache.cpp
+++ b/applypatch/freecache.cpp
@@ -16,10 +16,12 @@
#include <dirent.h>
#include <errno.h>
+#include <inttypes.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
+#include <sys/statfs.h>
#include <unistd.h>
#include <algorithm>
@@ -130,6 +132,24 @@
return std::numeric_limits<unsigned int>::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<int64_t>(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 @@
}
bool RemoveFilesInDirectory(size_t bytes_needed, const std::string& dirname,
- const std::function<size_t(const std::string&)>& space_checker) {
+ const std::function<int64_t(const std::string&)>& space_checker) {
struct stat st;
if (stat(dirname.c_str(), &st) == -1) {
PLOG(ERROR) << "Failed to stat " << dirname;
@@ -160,7 +180,11 @@
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 @@
}
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 92db59c..88659b8 100644
--- a/applypatch/include/applypatch/applypatch.h
+++ b/applypatch/include/applypatch/applypatch.h
@@ -40,10 +40,6 @@
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 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<size_t(const std::string&)>& space_checker);
+ const std::function<int64_t(const std::string&)>& space_checker);
#endif
diff --git a/tests/unit/applypatch_test.cpp b/tests/unit/applypatch_test.cpp
index 5cc03bc..4b757fe 100644
--- a/tests/unit/applypatch_test.cpp
+++ b/tests/unit/applypatch_test.cpp
@@ -16,6 +16,7 @@
#include <dirent.h>
#include <fcntl.h>
+#include <inttypes.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/stat.h>
@@ -29,6 +30,7 @@
#include <vector>
#include <android-base/file.h>
+#include <android-base/logging.h>
#include <android-base/stringprintf.h>
#include <android-base/test_utils.h>
#include <android-base/unique_fd.h>
@@ -176,7 +178,7 @@
// 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<std::string> files = FindFilesInDir(dirname);
return PARTITION_SIZE - 4096 * files.size();
}
@@ -191,7 +193,7 @@
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 @@
ASSERT_EQ(4096 * 9, MockFreeSpaceChecker(mock_cache.path));
}
+TEST_F(FreeCacheTest, FreeCacheFreeSpaceCheckerError) {
+ std::vector<std::string> 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<std::string> files = { "file1", "file2" };
AddFilesToDir(mock_cache.path, files);
@@ -209,7 +221,7 @@
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 @@
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 @@
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 @@
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);
}));