[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

5 files changed, 41 insertions, 57 deletions

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 06418721955b..dc05cb69d4c5 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -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()) {
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 9d40baff2226..0a18e21e9d6d 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -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);
diff --git a/services/incremental/ServiceWrappers.cpp b/services/incremental/ServiceWrappers.cpp
index 0b044c260160..32aa849c03b9 100644
--- a/services/incremental/ServiceWrappers.cpp
+++ b/services/incremental/ServiceWrappers.cpp
@@ -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);
     }
diff --git a/services/incremental/ServiceWrappers.h b/services/incremental/ServiceWrappers.h
index 39f8541c64c4..6b0f59e9c3e5 100644
--- a/services/incremental/ServiceWrappers.h
+++ b/services/incremental/ServiceWrappers.h
@@ -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;
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 8a0605348aab..7a8560221beb 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -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 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));
-    }
+    auto dir_path = std::string(first) + "/" + std::string(second) + "/" + std::string(third);
+
+    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);
 }