[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
This commit is contained in:
Yurii Zubrytskyi
2020-04-22 13:59:06 -07:00
parent f315786c67
commit efebb45ad7
5 changed files with 40 additions and 56 deletions

View File

@@ -212,6 +212,7 @@ auto IncrementalService::IncFsMount::makeStorage(StorageId id) -> StorageMap::it
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 @@ int IncrementalService::bind(StorageId storage, std::string_view source, std::st
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 @@ int IncrementalService::unbind(StorageId storage, std::string_view target) {
}
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 @@ std::string IncrementalService::normalizePathToStorageLocked(
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 @@ std::string IncrementalService::normalizePathToStorage(const IncrementalService:
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::makeFile(StorageId storage, std::string_view path, int m
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 @@ int IncrementalService::makeDirs(StorageId storageId, std::string_view path, int
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 @@ int IncrementalService::link(StorageId sourceStorageId, std::string_view oldPath
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::link(StorageId sourceStorageId, std::string_view oldPath
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 @@ RawMetadata IncrementalService::getMetadata(StorageId storage, std::string_view
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 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_
}
// 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 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_
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()) {

View File

@@ -280,12 +280,12 @@ private:
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);

View File

@@ -135,6 +135,9 @@ public:
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);
}

View File

@@ -81,6 +81,7 @@ public:
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;

View File

@@ -270,6 +270,8 @@ public:
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 @@ TEST_F(IncrementalServiceTest, testMakeDirectories) {
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);
}