IOS/FS: Fix CreateFullPath to not create directories that already exist

This fixes CreateFullPath to not create directories when it is known that they already exist, instead of calling CreateDirectory anyway and checking if the error is AlreadyExists. (That doesn't work now that we have an accurate implementation of CreateDirectory that performs permission checks before checking for existence.) I'm not sure what I was thinking when I wrote that function. Also adds some tests for CreateFullPath.
Ben754 · Jan 30, 2020 · bbc8631 · bbc8631
1 parent 4f01dad
commit bbc8631
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git a/Source/Core/Core/IOS/FS/FileSystem.cpp b/Source/Core/Core/IOS/FS/FileSystem.cpp
@@ -115,9 +115,12 @@ ResultCode FileSystem::CreateFullPath(Uid uid, Gid gid, const std::string& path,
     if (metadata && metadata->is_file)
       return ResultCode::Invalid;
 
-    const ResultCode result = CreateDirectory(uid, gid, subpath, attribute, modes);
-    if (result != ResultCode::Success && result != ResultCode::AlreadyExists)
-      return result;
+    if (!metadata)
+    {
+      const ResultCode result = CreateDirectory(uid, gid, subpath, attribute, modes);
+      if (result != ResultCode::Success)
+        return result;
+    }
 
     ++position;
   }

diff --git a/Source/UnitTests/Core/IOS/FS/FileSystemTest.cpp b/Source/UnitTests/Core/IOS/FS/FileSystemTest.cpp
@@ -434,3 +434,21 @@ TEST_F(FileSystemTest, ReadDirectoryOrdering)
   ASSERT_EQ(result->size(), file_names.size());
   EXPECT_TRUE(std::equal(result->begin(), result->end(), file_names.rbegin()));
 }
+
+TEST_F(FileSystemTest, CreateFullPath)
+{
+  ASSERT_EQ(m_fs->CreateFullPath(Uid{0}, Gid{0}, "/tmp/a/b/c/d", 0, modes), ResultCode::Success);
+
+  // Parent directories should be created by CreateFullPath.
+  for (const std::string& path : {"/tmp", "/tmp/a", "/tmp/a/b", "/tmp/a/b/c"})
+    EXPECT_TRUE(m_fs->ReadDirectory(Uid{0}, Gid{0}, path).Succeeded());
+
+  // If parent directories already exist, the call should still succeed.
+  EXPECT_EQ(m_fs->CreateFullPath(Uid{0}, Gid{0}, "/tmp/a/b/c/d", 0, modes), ResultCode::Success);
+
+  // If parent directories already exist and are owned by a different user,
+  // CreateFullPath should still succeed.
+  // See https://github.com/dolphin-emu/dolphin/pull/8593
+  EXPECT_EQ(m_fs->CreateFullPath(Uid{0x1000}, Gid{1}, "/shared2/wc24/mbox/Readme.txt", 0, modes),
+            ResultCode::Success);
+}