Merge pull request #8539 from leoetlino/fs-accuracy

IOS/FS: Reimplement many functions in a more accurate way

Source/Core/Core/IOS/ES/NandUtils.cpp

@@ -109,12 +109,6 @@ static std::vector<u64> GetTitlesInTitleOrImport(FS::FileSystem* fs, const std::
     }
   }
 
-  // On a real Wii, the title list is not in any particular order. However, because of how
-  // the flash filesystem works, titles such as 1-2 are *never* in the first position.
-  // We must keep this behaviour, or some versions of the System Menu may break.
-
-  std::sort(title_ids.begin(), title_ids.end(), std::greater<>());
-
   return title_ids;
 }
 

Source/Core/Core/IOS/FS/FileSystem.cpp

@@ -11,6 +11,24 @@
 
 namespace IOS::HLE::FS
 {
+bool IsValidPath(std::string_view path)
+{
+  return path == "/" || IsValidNonRootPath(path);
+}
+
+bool IsValidNonRootPath(std::string_view path)
+{
+  return path.length() > 1 && path.length() <= MaxPathLength && path[0] == '/' &&
+         path.back() != '/';
+}
+
+SplitPathResult SplitPathAndBasename(std::string_view path)
+{
+  const auto last_separator = path.rfind('/');
+  return {std::string(path.substr(0, std::max<size_t>(1, last_separator))),
+          std::string(path.substr(last_separator + 1))};
+}
+
 std::unique_ptr<FileSystem> MakeFileSystem(Location location)
 {
   const std::string nand_root =
@@ -66,12 +84,6 @@ Result<FileStatus> FileHandle::GetStatus() const
   return m_fs->GetFileStatus(*m_fd);
 }
 
-void FileSystem::Init()
-{
-  if (Delete(0, 0, "/tmp") == ResultCode::Success)
-    CreateDirectory(0, 0, "/tmp", 0, {Mode::ReadWrite, Mode::ReadWrite, Mode::ReadWrite});
-}
-
 Result<FileHandle> FileSystem::CreateAndOpenFile(Uid uid, Gid gid, const std::string& path,
                                                  Modes modes)
 {

Source/Core/Core/IOS/FS/FileSystem.h

@@ -7,6 +7,7 @@
 #include <memory>
 #include <optional>
 #include <string>
+#include <string_view>
 #include <vector>
 
 #ifdef _WIN32
@@ -76,6 +77,16 @@ struct Modes
 {
   Mode owner, group, other;
 };
+inline bool operator==(const Modes& lhs, const Modes& rhs)
+{
+  const auto fields = [](const Modes& obj) { return std::tie(obj.owner, obj.group, obj.other); };
+  return fields(lhs) == fields(rhs);
+}
+
+inline bool operator!=(const Modes& lhs, const Modes& rhs)
+{
+  return !(lhs == rhs);
+}
 
 struct Metadata
 {
@@ -111,6 +122,38 @@ struct FileStatus
   u32 size;
 };
 
+/// The maximum number of components a path can have.
+constexpr size_t MaxPathDepth = 8;
+/// The maximum number of characters a path can have.
+constexpr size_t MaxPathLength = 64;
+
+/// Returns whether a Wii path is valid.
+bool IsValidPath(std::string_view path);
+bool IsValidNonRootPath(std::string_view path);
+
+struct SplitPathResult
+{
+  std::string parent;
+  std::string file_name;
+};
+inline bool operator==(const SplitPathResult& lhs, const SplitPathResult& rhs)
+{
+  const auto fields = [](const SplitPathResult& obj) {
+    return std::tie(obj.parent, obj.file_name);
+  };
+  return fields(lhs) == fields(rhs);
+}
+
+inline bool operator!=(const SplitPathResult& lhs, const SplitPathResult& rhs)
+{
+  return !(lhs == rhs);
+}
+
+/// Split a path into a parent path and the file name. Takes a *valid non-root* path.
+///
+/// Example: /shared2/sys/SYSCONF => {/shared2/sys, SYSCONF}
+SplitPathResult SplitPathAndBasename(std::string_view path);
+
 class FileSystem;
 class FileHandle final
 {
@@ -196,9 +239,6 @@ public:
   virtual Result<NandStats> GetNandStats() = 0;
   /// Get usage information about a directory (used cluster and inode counts).
   virtual Result<DirectoryStats> GetDirectoryStats(const std::string& path) = 0;
-
-protected:
-  void Init();
 };
 
 template <typename T>

Source/Core/Core/IOS/FS/FileSystemProxy.cpp

@@ -16,6 +16,7 @@
 #include "Core/HW/Memmap.h"
 #include "Core/HW/SystemTimers.h"
 #include "Core/IOS/FS/FileSystem.h"
+#include "Core/IOS/Uids.h"
 
 namespace IOS::HLE::Device
 {
@@ -37,6 +38,11 @@ constexpr size_t CLUSTER_DATA_SIZE = 0x4000;
 
 FS::FS(Kernel& ios, const std::string& device_name) : Device(ios, device_name)
 {
+  if (ios.GetFS()->Delete(PID_KERNEL, PID_KERNEL, "/tmp") == ResultCode::Success)
+  {
+    ios.GetFS()->CreateDirectory(PID_KERNEL, PID_KERNEL, "/tmp", 0,
+                                 {Mode::ReadWrite, Mode::ReadWrite, Mode::ReadWrite});
+  }
 }
 
 void FS::DoState(PointerWrap& p)

Source/Core/Core/IOS/FS/HostBackend/FS.h

@@ -58,6 +58,20 @@ public:
   Result<DirectoryStats> GetDirectoryStats(const std::string& path) override;
 
 private:
+  struct FstEntry
+  {
+    bool CheckPermission(Uid uid, Gid gid, Mode requested_mode) const;
+
+    std::string name;
+    Metadata data{};
+    /// Children of this FST entry. Only valid for directories.
+    ///
+    /// We use a vector rather than a list here because iterating over children
+    /// happens a lot more often than removals.
+    /// Newly created entries are added at the end.
+    std::vector<FstEntry> children;
+  };
+
   struct Handle
   {
     bool opened = false;
@@ -73,6 +87,29 @@ private:
   std::string BuildFilename(const std::string& wii_path) const;
   std::shared_ptr<File::IOFile> OpenHostFile(const std::string& host_path);
 
+  ResultCode CreateFileOrDirectory(Uid uid, Gid gid, const std::string& path,
+                                   FileAttribute attribute, Modes modes, bool is_file);
+  bool IsFileOpened(const std::string& path) const;
+  bool IsDirectoryInUse(const std::string& path) const;
+
+  std::string GetFstFilePath() const;
+  void ResetFst();
+  void LoadFst();
+  void SaveFst();
+  /// Get the FST entry for a file (or directory).
+  /// Automatically creates fallback entries for parents if they do not exist.
+  /// Returns nullptr if the path is invalid or the file does not exist.
+  FstEntry* GetFstEntryForPath(const std::string& path);
+
+  /// FST entry for the filesystem root.
+  ///
+  /// Note that unlike a real Wii's FST, ours is the single source of truth only for
+  /// filesystem metadata and ordering. File existence must be checked by querying
+  /// the host filesystem.
+  /// The reasons for this design are twofold: existing users do not have a FST
+  /// and we do not want FS to break if the user adds or removes files in their
+  /// filesystem root manually.
+  FstEntry m_root_entry{};
   std::string m_root_path;
   std::map<std::string, std::weak_ptr<File::IOFile>> m_open_files;
   std::array<Handle, 16> m_handles{};

Source/Core/Core/NetPlayClient.cpp

@@ -904,8 +904,8 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet)
       {
         auto buffer = DecompressPacketIntoBuffer(packet);
 
-        temp_fs->CreateDirectory(IOS::PID_KERNEL, IOS::PID_KERNEL, "/shared2/menu/FaceLib", 0,
-                                 fs_modes);
+        temp_fs->CreateFullPath(IOS::PID_KERNEL, IOS::PID_KERNEL, "/shared2/menu/FaceLib/", 0,
+                                fs_modes);
         auto file = temp_fs->CreateAndOpenFile(IOS::PID_KERNEL, IOS::PID_KERNEL,
                                                Common::GetMiiDatabasePath(), fs_modes);
 
@@ -924,8 +924,8 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet)
       {
         u64 title_id = Common::PacketReadU64(packet);
         titles.push_back(title_id);
-        temp_fs->CreateDirectory(IOS::PID_KERNEL, IOS::PID_KERNEL,
-                                 Common::GetTitleDataPath(title_id), 0, fs_modes);
+        temp_fs->CreateFullPath(IOS::PID_KERNEL, IOS::PID_KERNEL,
+                                Common::GetTitleDataPath(title_id) + '/', 0, fs_modes);
         auto save = WiiSave::MakeNandStorage(temp_fs.get(), title_id);
 
         bool exists;

Source/Core/Core/WiiRoot.cpp

@@ -34,9 +34,8 @@ static std::string s_temp_wii_root;
 
 static void CopySave(FS::FileSystem* source, FS::FileSystem* dest, const u64 title_id)
 {
-  dest->CreateDirectory(IOS::PID_KERNEL, IOS::PID_KERNEL, Common::GetTitleDataPath(title_id), 0,
-                        {IOS::HLE::FS::Mode::ReadWrite, IOS::HLE::FS::Mode::ReadWrite,
-                         IOS::HLE::FS::Mode::ReadWrite});
+  dest->CreateFullPath(IOS::PID_KERNEL, IOS::PID_KERNEL, Common::GetTitleDataPath(title_id) + '/',
+                       0, {FS::Mode::ReadWrite, FS::Mode::ReadWrite, FS::Mode::ReadWrite});
   const auto source_save = WiiSave::MakeNandStorage(source, title_id);
   const auto dest_save = WiiSave::MakeNandStorage(dest, title_id);
   WiiSave::Copy(source_save.get(), dest_save.get());
@@ -49,9 +48,8 @@ static bool CopyNandFile(FS::FileSystem* source_fs, const std::string& source_fi
   if (last_slash != std::string::npos && last_slash > 0)
   {
     const std::string dir = dest_file.substr(0, last_slash);
-    dest_fs->CreateDirectory(IOS::PID_KERNEL, IOS::PID_KERNEL, dir, 0,
-                             {IOS::HLE::FS::Mode::ReadWrite, IOS::HLE::FS::Mode::ReadWrite,
-                              IOS::HLE::FS::Mode::ReadWrite});
+    dest_fs->CreateFullPath(IOS::PID_KERNEL, IOS::PID_KERNEL, dir + '/', 0,
+                            {FS::Mode::ReadWrite, FS::Mode::ReadWrite, FS::Mode::ReadWrite});
   }
 
   auto source_handle =
@@ -190,7 +188,7 @@ static bool CopySysmenuFilesToFS(FS::FileSystem* fs, const std::string& host_sou
 
     if (entry.isDirectory)
     {
-      fs->CreateDirectory(IOS::SYSMENU_UID, IOS::SYSMENU_GID, nand_path, 0, public_modes);
+      fs->CreateFullPath(IOS::SYSMENU_UID, IOS::SYSMENU_GID, nand_path + '/', 0, public_modes);
       if (!CopySysmenuFilesToFS(fs, host_path, nand_path))
         return false;
     }
@@ -259,12 +257,8 @@ void CleanUpWiiFileSystemContents()
 
     // FS won't write the save if the directory doesn't exist
     const std::string title_path = Common::GetTitleDataPath(title_id);
-    if (!configured_fs->GetMetadata(IOS::PID_KERNEL, IOS::PID_KERNEL, title_path))
-    {
-      configured_fs->CreateDirectory(IOS::PID_KERNEL, IOS::PID_KERNEL, title_path, 0,
-                                     {IOS::HLE::FS::Mode::ReadWrite, IOS::HLE::FS::Mode::ReadWrite,
-                                      IOS::HLE::FS::Mode::ReadWrite});
-    }
+    configured_fs->CreateFullPath(IOS::PID_KERNEL, IOS::PID_KERNEL, title_path + '/', 0,
+                                  {FS::Mode::ReadWrite, FS::Mode::ReadWrite, FS::Mode::ReadWrite});
 
     const auto user_save = WiiSave::MakeNandStorage(configured_fs.get(), title_id);
 

Source/UnitTests/Core/IOS/FS/FileSystemTest.cpp

@@ -2,8 +2,10 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
+#include <algorithm>
 #include <array>
 #include <memory>
+#include <optional>
 #include <string>
 
 #include <gtest/gtest.h>
@@ -39,6 +41,41 @@ private:
   std::string m_profile_path;
 };
 
+TEST(FileSystem, BasicPathValidity)
+{
+  EXPECT_TRUE(IsValidPath("/"));
+  EXPECT_FALSE(IsValidNonRootPath("/"));
+
+  EXPECT_TRUE(IsValidNonRootPath("/shared2/sys/SYSCONF"));
+  EXPECT_TRUE(IsValidNonRootPath("/shared2/sys"));
+  EXPECT_TRUE(IsValidNonRootPath("/shared2"));
+
+  // Paths must start with /.
+  EXPECT_FALSE(IsValidNonRootPath("\\test"));
+  // Paths must not end with /.
+  EXPECT_FALSE(IsValidNonRootPath("/shared2/sys/"));
+  // Paths must not be longer than 64 characters.
+  EXPECT_FALSE(IsValidPath(
+      "/abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"));
+}
+
+TEST(FileSystem, PathSplitting)
+{
+  SplitPathResult result;
+
+  result = {"/shared1", "00000042.app"};
+  EXPECT_EQ(SplitPathAndBasename("/shared1/00000042.app"), result);
+
+  result = {"/shared2/sys", "SYSCONF"};
+  EXPECT_EQ(SplitPathAndBasename("/shared2/sys/SYSCONF"), result);
+
+  result = {"/shared2", "sys"};
+  EXPECT_EQ(SplitPathAndBasename("/shared2/sys"), result);
+
+  result = {"/", "shared2"};
+  EXPECT_EQ(SplitPathAndBasename("/shared2"), result);
+}
+
 TEST_F(FileSystemTest, EssentialDirectories)
 {
   for (const std::string& path :
@@ -52,41 +89,59 @@ TEST_F(FileSystemTest, CreateFile)
 {
   const std::string PATH = "/tmp/f";
 
-  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, PATH, 0, modes), ResultCode::Success);
+  constexpr u8 ArbitraryAttribute = 0xE1;
+
+  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, PATH, ArbitraryAttribute, modes), ResultCode::Success);
 
   const Result<Metadata> stats = m_fs->GetMetadata(Uid{0}, Gid{0}, PATH);
   ASSERT_TRUE(stats.Succeeded());
   EXPECT_TRUE(stats->is_file);
   EXPECT_EQ(stats->size, 0u);
-  // TODO: After we start saving metadata correctly, check the UID, GID, permissions
-  // as well (issue 10234).
+  EXPECT_EQ(stats->uid, 0);
+  EXPECT_EQ(stats->gid, 0);
+  EXPECT_EQ(stats->modes, modes);
+  EXPECT_EQ(stats->attribute, ArbitraryAttribute);
 
   ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, PATH, 0, modes), ResultCode::AlreadyExists);
 
   const Result<std::vector<std::string>> tmp_files = m_fs->ReadDirectory(Uid{0}, Gid{0}, "/tmp");
   ASSERT_TRUE(tmp_files.Succeeded());
   EXPECT_EQ(std::count(tmp_files->begin(), tmp_files->end(), "f"), 1u);
+
+  // Test invalid paths
+  // Unprintable characters
+  EXPECT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/tmp/tes\1t", 0, modes), ResultCode::Invalid);
+  EXPECT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/tmp/te\x7fst", 0, modes), ResultCode::Invalid);
+  // Paths with too many components are not rejected for files.
+  EXPECT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/1/2/3/4/5/6/7/8/9", 0, modes), ResultCode::NotFound);
 }
 
 TEST_F(FileSystemTest, CreateDirectory)
 {
   const std::string PATH = "/tmp/d";
 
-  ASSERT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, PATH, 0, modes), ResultCode::Success);
+  constexpr u8 ArbitraryAttribute = 0x20;
+
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, PATH, ArbitraryAttribute, modes),
+            ResultCode::Success);
 
   const Result<Metadata> stats = m_fs->GetMetadata(Uid{0}, Gid{0}, PATH);
   ASSERT_TRUE(stats.Succeeded());
   EXPECT_FALSE(stats->is_file);
-  // TODO: After we start saving metadata correctly, check the UID, GID, permissions
-  // as well (issue 10234).
+  EXPECT_EQ(stats->uid, 0);
+  EXPECT_EQ(stats->gid, 0);
+  EXPECT_EQ(stats->modes, modes);
+  EXPECT_EQ(stats->attribute, ArbitraryAttribute);
 
   const Result<std::vector<std::string>> children = m_fs->ReadDirectory(Uid{0}, Gid{0}, PATH);
   ASSERT_TRUE(children.Succeeded());
   EXPECT_TRUE(children->empty());
 
-  // TODO: uncomment this after the FS code is fixed to return AlreadyExists.
-  // EXPECT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, PATH, 0, Mode::Read, Mode::None, Mode::None),
-  //           ResultCode::AlreadyExists);
+  EXPECT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, PATH, 0, modes), ResultCode::AlreadyExists);
+
+  // Paths with too many components should be rejected.
+  EXPECT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, "/1/2/3/4/5/6/7/8/9", 0, modes),
+            ResultCode::TooManyPathComponents);
 }
 
 TEST_F(FileSystemTest, Delete)
@@ -94,6 +149,25 @@ TEST_F(FileSystemTest, Delete)
   EXPECT_TRUE(m_fs->ReadDirectory(Uid{0}, Gid{0}, "/tmp").Succeeded());
   EXPECT_EQ(m_fs->Delete(Uid{0}, Gid{0}, "/tmp"), ResultCode::Success);
   EXPECT_EQ(m_fs->ReadDirectory(Uid{0}, Gid{0}, "/tmp").Error(), ResultCode::NotFound);
+
+  // Test recursive directory deletion.
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, "/sys/1", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, "/sys/1/2", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/sys/1/2/3", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/sys/1/2/4", 0, modes), ResultCode::Success);
+
+  // Leave a file open. Deletion should fail while the file is in use.
+  auto handle = std::make_optional(m_fs->OpenFile(Uid{0}, Gid{0}, "/sys/1/2/3", Mode::Read));
+  ASSERT_TRUE(handle->Succeeded());
+  EXPECT_EQ(m_fs->Delete(Uid{0}, Gid{0}, "/sys/1/2/3"), ResultCode::InUse);
+  // A directory that contains a file that is in use is considered to be in use,
+  // so this should fail too.
+  EXPECT_EQ(m_fs->Delete(Uid{0}, Gid{0}, "/sys/1"), ResultCode::InUse);
+
+  // With the handle closed, both of these should work:
+  handle.reset();
+  EXPECT_EQ(m_fs->Delete(Uid{0}, Gid{0}, "/sys/1/2/3"), ResultCode::Success);
+  EXPECT_EQ(m_fs->Delete(Uid{0}, Gid{0}, "/sys/1"), ResultCode::Success);
 }
 
 TEST_F(FileSystemTest, Rename)
@@ -104,6 +178,14 @@ TEST_F(FileSystemTest, Rename)
 
   EXPECT_EQ(m_fs->ReadDirectory(Uid{0}, Gid{0}, "/tmp").Error(), ResultCode::NotFound);
   EXPECT_TRUE(m_fs->ReadDirectory(Uid{0}, Gid{0}, "/test").Succeeded());
+
+  // Rename /test back to /tmp.
+  EXPECT_EQ(m_fs->Rename(Uid{0}, Gid{0}, "/test", "/tmp"), ResultCode::Success);
+
+  // Create a file called /tmp/f1, and rename it to /tmp/f2.
+  // This should not work; file name changes are not allowed for files.
+  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/tmp/f1", 0, modes), ResultCode::Success);
+  EXPECT_EQ(m_fs->Rename(Uid{0}, Gid{0}, "/tmp/f1", "/tmp/f2"), ResultCode::Invalid);
 }
 
 TEST_F(FileSystemTest, RenameWithExistingTargetDirectory)
@@ -124,26 +206,29 @@ TEST_F(FileSystemTest, RenameWithExistingTargetDirectory)
 
 TEST_F(FileSystemTest, RenameWithExistingTargetFile)
 {
+  const std::string source_path = "/sys/f2";
+  const std::string dest_path = "/tmp/f2";
+
   // Create the test source file and write some data (so that we can check its size later on).
-  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/tmp/f1", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, source_path, 0, modes), ResultCode::Success);
   const std::vector<u8> TEST_DATA{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}};
   std::vector<u8> read_buffer(TEST_DATA.size());
   {
-    const Result<FileHandle> file = m_fs->OpenFile(Uid{0}, Gid{0}, "/tmp/f1", Mode::ReadWrite);
+    const Result<FileHandle> file = m_fs->OpenFile(Uid{0}, Gid{0}, source_path, Mode::ReadWrite);
     ASSERT_TRUE(file.Succeeded());
     ASSERT_TRUE(file->Write(TEST_DATA.data(), TEST_DATA.size()).Succeeded());
   }
 
   // Create the test target file and leave it empty.
-  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/tmp/f2", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, dest_path, 0, modes), ResultCode::Success);
 
-  // Rename f1 to f2 and check that f1 replaced f2.
-  EXPECT_EQ(m_fs->Rename(Uid{0}, Gid{0}, "/tmp/f1", "/tmp/f2"), ResultCode::Success);
+  // Rename /sys/f2 to /tmp/f2 and check that f1 replaced f2.
+  EXPECT_EQ(m_fs->Rename(Uid{0}, Gid{0}, source_path, dest_path), ResultCode::Success);
 
-  ASSERT_FALSE(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/f1").Succeeded());
-  EXPECT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/f1").Error(), ResultCode::NotFound);
+  ASSERT_FALSE(m_fs->GetMetadata(Uid{0}, Gid{0}, source_path).Succeeded());
+  EXPECT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, source_path).Error(), ResultCode::NotFound);
 
-  const Result<Metadata> metadata = m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/f2");
+  const Result<Metadata> metadata = m_fs->GetMetadata(Uid{0}, Gid{0}, dest_path);
   ASSERT_TRUE(metadata.Succeeded());
   EXPECT_TRUE(metadata->is_file);
   EXPECT_EQ(metadata->size, TEST_DATA.size());
@@ -325,3 +410,27 @@ TEST_F(FileSystemTest, ReadDirectoryOnFile)
   ASSERT_FALSE(result.Succeeded());
   EXPECT_EQ(result.Error(), ResultCode::Invalid);
 }
+
+TEST_F(FileSystemTest, ReadDirectoryOrdering)
+{
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, "/tmp/o", 0, modes), ResultCode::Success);
+
+  // Randomly generated file names in no particular order.
+  const std::array<std::string, 5> file_names{{
+      "Rkj62lGwHp",
+      "XGDQTDJMea",
+      "1z5M43WeFw",
+      "YAY39VuMRd",
+      "hxJ86nkoBX",
+  }};
+  // Create the files.
+  for (const auto& name : file_names)
+    ASSERT_EQ(m_fs->CreateFile(Uid{0}, Gid{0}, "/tmp/o/" + name, 0, modes), ResultCode::Success);
+
+  // Verify that ReadDirectory returns a file list that is ordered by descending creation date
+  // (issue 10234).
+  const Result<std::vector<std::string>> result = m_fs->ReadDirectory(Uid{0}, Gid{0}, "/tmp/o");
+  ASSERT_TRUE(result.Succeeded());
+  ASSERT_EQ(result->size(), file_names.size());
+  EXPECT_TRUE(std::equal(result->begin(), result->end(), file_names.rbegin()));
+}

Tools/print-fs-fst.py

@@ -0,0 +1,58 @@
+import argparse
+import struct
+
+def read_entry(f) -> dict:
+    name = struct.unpack_from("12s", f.read(12))[0]
+    uid = struct.unpack_from(">I", f.read(4))[0]
+    gid = struct.unpack_from(">H", f.read(2))[0]
+    is_file = struct.unpack_from("?", f.read(1))[0]
+    modes = struct.unpack_from("BBB", f.read(3))
+    attr = struct.unpack_from("B", f.read(2))[0]
+    x3 = struct.unpack_from(">I", f.read(4))[0]
+    num_children = struct.unpack_from(">I", f.read(4))[0]
+
+    children = []
+    for i in range(num_children):
+        children.append(read_entry(f))
+
+    return {
+        "name": name,
+        "uid": uid,
+        "gid": gid,
+        "is_file": is_file,
+        "modes": modes,
+        "attr": attr,
+        "x3": x3,
+        "children": children,
+    }
+
+COLOR_RESET = "\x1b[0;00m"
+BOLD = "\x1b[0;37m"
+COLOR_BLUE = "\x1b[1;34m"
+COLOR_GREEN = "\x1b[0;32m"
+
+def print_entry(entry, indent) -> None:
+    mode_str = {0: "--", 1: "r-", 2: "-w", 3: "rw"}
+
+    sp = ' ' * indent
+    color = BOLD if entry["is_file"] else COLOR_BLUE
+
+    owner = f"{COLOR_GREEN}{entry['uid']:04x}{COLOR_RESET}:{entry['gid']:04x}"
+    attrs = f"{''.join(mode_str[mode] for mode in entry['modes'])}"
+    other_attrs = f"{entry['attr']} {entry['x3']}"
+
+    print(f"{sp}{color}{entry['name'].decode()}{COLOR_RESET} [{owner} {attrs} {other_attrs}]")
+    for child in entry["children"]:
+        print_entry(child, indent + 2)
+
+def main() -> None:
+    parser = argparse.ArgumentParser(description="Prints a FST in a tree-like format.")
+    parser.add_argument("file")
+    args = parser.parse_args()
+
+    with open(args.file, "rb") as f:
+        root = read_entry(f)
+
+    print_entry(root, 0)
+
+main()