[incfs] Use the new IncFs_MakeDirs() function
This gets rid of annoying warnings in logcat about not being
able to create a directory and then directory already existing
Bug: 153704006
Test: atest IncrementalServiceTest
Test: adb install megacity.apk
Change-Id: Ib718960287f93cb383c06c9b9e3d0abf1ec42916
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 0641872..dc05cb6 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -212,6 +212,7 @@
template <class Func>
static auto makeCleanup(Func&& f) {
auto deleter = [f = std::move(f)](auto) { f(); };
+ // &f is a dangling pointer here, but we actually never use it as deleter moves it in.
return std::unique_ptr<Func, decltype(deleter)>(&f, std::move(deleter));
}
@@ -719,7 +720,7 @@
LOG(ERROR) << "no storage";
return -EINVAL;
}
- std::string normSource = normalizePathToStorageLocked(ifs, storageInfo, source);
+ std::string normSource = normalizePathToStorageLocked(*ifs, storageInfo, source);
if (normSource.empty()) {
LOG(ERROR) << "invalid source path";
return -EINVAL;
@@ -773,7 +774,7 @@
}
std::string IncrementalService::normalizePathToStorageLocked(
- const IfsMountPtr& incfs, IncFsMount::StorageMap::iterator storageIt,
+ const IncFsMount& incfs, IncFsMount::StorageMap::const_iterator storageIt,
std::string_view path) const {
if (!path::isAbsolute(path)) {
return path::normalize(path::join(storageIt->second.name, path));
@@ -783,19 +784,18 @@
return normPath;
}
// not that easy: need to find if any of the bind points match
- const auto bindIt = findParentPath(incfs->bindPoints, normPath);
- if (bindIt == incfs->bindPoints.end()) {
+ const auto bindIt = findParentPath(incfs.bindPoints, normPath);
+ if (bindIt == incfs.bindPoints.end()) {
return {};
}
return path::join(bindIt->second.sourceDir, path::relativize(bindIt->first, normPath));
}
-std::string IncrementalService::normalizePathToStorage(const IncrementalService::IfsMountPtr& ifs,
- StorageId storage,
+std::string IncrementalService::normalizePathToStorage(const IncFsMount& ifs, StorageId storage,
std::string_view path) const {
- std::unique_lock l(ifs->lock);
- const auto storageInfo = ifs->storages.find(storage);
- if (storageInfo == ifs->storages.end()) {
+ std::unique_lock l(ifs.lock);
+ const auto storageInfo = ifs.storages.find(storage);
+ if (storageInfo == ifs.storages.end()) {
return {};
}
return normalizePathToStorageLocked(ifs, storageInfo, path);
@@ -804,7 +804,7 @@
int IncrementalService::makeFile(StorageId storage, std::string_view path, int mode, FileId id,
incfs::NewFileParams params) {
if (auto ifs = getIfs(storage)) {
- std::string normPath = normalizePathToStorage(ifs, storage, path);
+ std::string normPath = normalizePathToStorage(*ifs, storage, path);
if (normPath.empty()) {
LOG(ERROR) << "Internal error: storageId " << storage
<< " failed to normalize: " << path;
@@ -822,7 +822,7 @@
int IncrementalService::makeDir(StorageId storageId, std::string_view path, int mode) {
if (auto ifs = getIfs(storageId)) {
- std::string normPath = normalizePathToStorage(ifs, storageId, path);
+ std::string normPath = normalizePathToStorage(*ifs, storageId, path);
if (normPath.empty()) {
return -EINVAL;
}
@@ -836,21 +836,16 @@
if (!ifs) {
return -EINVAL;
}
+ return makeDirs(*ifs, storageId, path, mode);
+}
+
+int IncrementalService::makeDirs(const IncFsMount& ifs, StorageId storageId, std::string_view path,
+ int mode) {
std::string normPath = normalizePathToStorage(ifs, storageId, path);
if (normPath.empty()) {
return -EINVAL;
}
- auto err = mIncFs->makeDir(ifs->control, normPath, mode);
- if (err == -EEXIST) {
- return 0;
- }
- if (err != -ENOENT) {
- return err;
- }
- if (auto err = makeDirs(storageId, path::dirname(normPath), mode)) {
- return err;
- }
- return mIncFs->makeDir(ifs->control, normPath, mode);
+ return mIncFs->makeDirs(ifs.control, normPath, mode);
}
int IncrementalService::link(StorageId sourceStorageId, std::string_view oldPath,
@@ -864,8 +859,8 @@
return -EINVAL;
}
l.unlock();
- std::string normOldPath = normalizePathToStorage(ifsSrc, sourceStorageId, oldPath);
- std::string normNewPath = normalizePathToStorage(ifsSrc, destStorageId, newPath);
+ std::string normOldPath = normalizePathToStorage(*ifsSrc, sourceStorageId, oldPath);
+ std::string normNewPath = normalizePathToStorage(*ifsSrc, destStorageId, newPath);
if (normOldPath.empty() || normNewPath.empty()) {
LOG(ERROR) << "Invalid paths in link(): " << normOldPath << " | " << normNewPath;
return -EINVAL;
@@ -875,7 +870,7 @@
int IncrementalService::unlink(StorageId storage, std::string_view path) {
if (auto ifs = getIfs(storage)) {
- std::string normOldPath = normalizePathToStorage(ifs, storage, path);
+ std::string normOldPath = normalizePathToStorage(*ifs, storage, path);
return mIncFs->unlink(ifs->control, normOldPath);
}
return -EINVAL;
@@ -960,7 +955,7 @@
if (!ifs) {
return {};
}
- const auto normPath = normalizePathToStorage(ifs, storage, path);
+ const auto normPath = normalizePathToStorage(*ifs, storage, path);
if (normPath.empty()) {
return {};
}
@@ -1363,7 +1358,7 @@
}
// First prepare target directories if they don't exist yet
- if (auto res = makeDirs(storage, libDirRelativePath, 0755)) {
+ if (auto res = makeDirs(*ifs, storage, libDirRelativePath, 0755)) {
LOG(ERROR) << "Failed to prepare target lib directory " << libDirRelativePath
<< " errno: " << res;
return false;
@@ -1401,7 +1396,7 @@
const auto libName = path::basename(fileName);
const auto targetLibPath = path::join(libDirRelativePath, libName);
- const auto targetLibPathAbsolute = normalizePathToStorage(ifs, storage, targetLibPath);
+ const auto targetLibPathAbsolute = normalizePathToStorage(*ifs, storage, targetLibPath);
// If the extract file already exists, skip
if (access(targetLibPathAbsolute.c_str(), F_OK) == 0) {
if (perfLoggingEnabled()) {
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 9d40baf..0a18e21 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -280,12 +280,12 @@
void deleteStorage(IncFsMount& ifs);
void deleteStorageLocked(IncFsMount& ifs, std::unique_lock<std::mutex>&& ifsLock);
MountMap::iterator getStorageSlotLocked();
- std::string normalizePathToStorage(const IfsMountPtr& incfs, StorageId storage,
+ std::string normalizePathToStorage(const IncFsMount& incfs, StorageId storage,
std::string_view path) const;
- std::string normalizePathToStorageLocked(const IfsMountPtr& incfs,
- IncFsMount::StorageMap::iterator storageIt,
+ std::string normalizePathToStorageLocked(const IncFsMount& incfs,
+ IncFsMount::StorageMap::const_iterator storageIt,
std::string_view path) const;
-
+ int makeDirs(const IncFsMount& ifs, StorageId storageId, std::string_view path, int mode);
binder::Status applyStorageParams(IncFsMount& ifs, bool enableReadLogs);
void registerAppOpsCallback(const std::string& packageName);
diff --git a/services/incremental/ServiceWrappers.cpp b/services/incremental/ServiceWrappers.cpp
index 0b044c2..32aa849 100644
--- a/services/incremental/ServiceWrappers.cpp
+++ b/services/incremental/ServiceWrappers.cpp
@@ -135,6 +135,9 @@
ErrorCode makeDir(const Control& control, std::string_view path, int mode) const final {
return incfs::makeDir(control, path, mode);
}
+ ErrorCode makeDirs(const Control& control, std::string_view path, int mode) const final {
+ return incfs::makeDirs(control, path, mode);
+ }
incfs::RawMetadata getMetadata(const Control& control, FileId fileid) const final {
return incfs::getMetadata(control, fileid);
}
diff --git a/services/incremental/ServiceWrappers.h b/services/incremental/ServiceWrappers.h
index 39f8541..6b0f59e 100644
--- a/services/incremental/ServiceWrappers.h
+++ b/services/incremental/ServiceWrappers.h
@@ -81,6 +81,7 @@
virtual ErrorCode makeFile(const Control& control, std::string_view path, int mode, FileId id,
incfs::NewFileParams params) const = 0;
virtual ErrorCode makeDir(const Control& control, std::string_view path, int mode) const = 0;
+ virtual ErrorCode makeDirs(const Control& control, std::string_view path, int mode) const = 0;
virtual incfs::RawMetadata getMetadata(const Control& control, FileId fileid) const = 0;
virtual incfs::RawMetadata getMetadata(const Control& control, std::string_view path) const = 0;
virtual FileId getFileId(const Control& control, std::string_view path) const = 0;
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 8a06053..7a85602 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -270,6 +270,8 @@
ErrorCode(const Control& control, std::string_view path, int mode, FileId id,
NewFileParams params));
MOCK_CONST_METHOD3(makeDir, ErrorCode(const Control& control, std::string_view path, int mode));
+ MOCK_CONST_METHOD3(makeDirs,
+ ErrorCode(const Control& control, std::string_view path, int mode));
MOCK_CONST_METHOD2(getMetadata, RawMetadata(const Control& control, FileId fileid));
MOCK_CONST_METHOD2(getMetadata, RawMetadata(const Control& control, std::string_view path));
MOCK_CONST_METHOD2(getFileId, FileId(const Control& control, std::string_view path));
@@ -723,31 +725,14 @@
auto first = "first"sv;
auto second = "second"sv;
auto third = "third"sv;
- auto parent_path = std::string(first) + "/" + std::string(second);
- auto dir_path = parent_path + "/" + std::string(third);
+ auto dir_path = std::string(first) + "/" + std::string(second) + "/" + std::string(third);
- auto checkArgFor = [&](std::string_view expected, std::string_view arg) {
- return arg.starts_with(mRootDir.path) && arg.ends_with("/mount/st_1_0/"s += expected);
- };
-
- {
- InSequence seq;
- EXPECT_CALL(*mIncFs,
- makeDir(_, Truly([&](auto arg) { return checkArgFor(dir_path, arg); }), _))
- .WillOnce(Return(-ENOENT));
- EXPECT_CALL(*mIncFs,
- makeDir(_, Truly([&](auto arg) { return checkArgFor(parent_path, arg); }), _))
- .WillOnce(Return(-ENOENT));
- EXPECT_CALL(*mIncFs,
- makeDir(_, Truly([&](auto arg) { return checkArgFor(first, arg); }), _))
- .WillOnce(Return(0));
- EXPECT_CALL(*mIncFs,
- makeDir(_, Truly([&](auto arg) { return checkArgFor(parent_path, arg); }), _))
- .WillOnce(Return(0));
- EXPECT_CALL(*mIncFs,
- makeDir(_, Truly([&](auto arg) { return checkArgFor(dir_path, arg); }), _))
- .WillOnce(Return(0));
- }
+ EXPECT_CALL(*mIncFs,
+ makeDirs(_, Truly([&](std::string_view arg) {
+ return arg.starts_with(mRootDir.path) &&
+ arg.ends_with("/mount/st_1_0/" + dir_path);
+ }),
+ _));
auto res = mIncrementalService->makeDirs(storageId, dir_path, 0555);
ASSERT_EQ(res, 0);
}
