Address review comments

src/core/file_sys/fs_filesystem.h

@@ -24,7 +24,7 @@ enum class OpenDirectoryMode : u64 {
 
     All = (Directory | File),
 
-    NotRequireFileSize = (1 << 31),
+    NotRequireFileSize = (1ULL << 31),
 };
 DECLARE_ENUM_FLAG_OPERATORS(OpenDirectoryMode)
 

src/core/file_sys/fs_path_utility.h

@@ -91,8 +91,12 @@ public:
     }
 
 #define DECLARE_PATH_FLAG_HANDLER(__WHICH__)                                                       \
-    constexpr bool Is##__WHICH__##Allowed() const { return (m_value & __WHICH__##Flag) != 0; }     \
+    constexpr bool Is##__WHICH__##Allowed() const {                                                \
-    constexpr void Allow##__WHICH__() { m_value |= __WHICH__##Flag; }
+        return (m_value & __WHICH__##Flag) != 0;                                                   \
+    }                                                                                              \
+    constexpr void Allow##__WHICH__() {                                                            \
+        m_value |= __WHICH__##Flag;                                                                \
+    }
 
     DECLARE_PATH_FLAG_HANDLER(WindowsPath)
     DECLARE_PATH_FLAG_HANDLER(RelativePath)
@@ -426,9 +430,10 @@ public:
         R_SUCCEED();
     }
 
-    static Result Normalize(char* dst, size_t* out_len, const char* path, size_t max_out_size,
+    static constexpr Result Normalize(char* dst, size_t* out_len, const char* path,
-                            bool is_windows_path, bool is_drive_relative_path,
+                                      size_t max_out_size, bool is_windows_path,
-                            bool allow_all_characters = false) {
+                                      bool is_drive_relative_path,
+                                      bool allow_all_characters = false) {
         // Use StringTraits names for remainder of scope
         using namespace StringTraits;
 

src/core/file_sys/fsa/fs_i_directory.h

@@ -16,14 +16,15 @@ namespace FileSys::Fsa {
 
 class IDirectory {
 public:
-    IDirectory(VirtualDir backend_, OpenDirectoryMode mode) : backend(std::move(backend_)) {
+    explicit IDirectory(VirtualDir backend_, OpenDirectoryMode mode)
+        : backend(std::move(backend_)) {
         // TODO(DarkLordZach): Verify that this is the correct behavior.
         // Build entry index now to save time later.
         if (True(mode & OpenDirectoryMode::Directory)) {
-            BuildEntryIndex(entries, backend->GetSubdirectories(), DirectoryEntryType::Directory);
+            BuildEntryIndex(backend->GetSubdirectories(), DirectoryEntryType::Directory);
         }
         if (True(mode & OpenDirectoryMode::File)) {
-            BuildEntryIndex(entries, backend->GetFiles(), DirectoryEntryType::File);
+            BuildEntryIndex(backend->GetFiles(), DirectoryEntryType::File);
         }
     }
     virtual ~IDirectory() {}
@@ -45,28 +46,29 @@ public:
     }
 
 private:
-    virtual Result DoRead(s64* out_count, DirectoryEntry* out_entries, s64 max_entries) {
+    Result DoRead(s64* out_count, DirectoryEntry* out_entries, s64 max_entries) {
         const u64 actual_entries =
             std::min(static_cast<u64>(max_entries), entries.size() - next_entry_index);
-        auto* begin = reinterpret_cast<u8*>(entries.data() + next_entry_index);
+        const auto* begin = reinterpret_cast<u8*>(entries.data() + next_entry_index);
+        const auto* end = reinterpret_cast<u8*>(entries.data() + next_entry_index + actual_entries);
+        const auto range_size = static_cast<std::size_t>(std::distance(begin, end));
 
         next_entry_index += actual_entries;
         *out_count = actual_entries;
 
-        out_entries = reinterpret_cast<DirectoryEntry*>(begin);
+        std::memcpy(out_entries, entries.data(), range_size);
 
         R_SUCCEED();
     }
 
-    virtual Result DoGetEntryCount(s64* out) {
+    Result DoGetEntryCount(s64* out) {
         *out = entries.size() - next_entry_index;
         R_SUCCEED();
     }
 
     // TODO: Remove this when VFS is gone
     template <typename T>
-    void BuildEntryIndex(std::vector<DirectoryEntry>& entries, const std::vector<T>& new_data,
+    void BuildEntryIndex(const std::vector<T>& new_data, DirectoryEntryType type) {
-                         DirectoryEntryType type) {
         entries.reserve(entries.size() + new_data.size());
 
         for (const auto& new_entry : new_data) {

src/core/file_sys/fsa/fs_i_file.h

@@ -16,7 +16,7 @@ namespace FileSys::Fsa {
 
 class IFile {
 public:
-    IFile(VirtualFile backend_) : backend(std::move(backend_)) {}
+    explicit IFile(VirtualFile backend_) : backend(std::move(backend_)) {}
     virtual ~IFile() {}
 
     Result Read(size_t* out, s64 offset, void* buffer, size_t size, const ReadOption& option) {
@@ -126,8 +126,10 @@ protected:
 private:
     Result DoRead(size_t* out, s64 offset, void* buffer, size_t size, const ReadOption& option) {
         std::vector<u8> output = backend->ReadBytes(size, offset);
+
         *out = output.size();
-        buffer = output.data();
+        std::memcpy(buffer, output.data(), size);
+
         R_SUCCEED();
     }
 

src/core/file_sys/fsa/fs_i_filesystem.h

@@ -15,7 +15,7 @@ namespace FileSys::Fsa {
 class IFile;
 class IDirectory;
 
-enum class QueryId {
+enum class QueryId : u32 {
     SetConcatenationFileAttribute = 0,
     UpdateMac = 1,
     IsSignedSystemPartitionOnSdCardValid = 2,
@@ -24,7 +24,7 @@ enum class QueryId {
 
 class IFileSystem {
 public:
-    IFileSystem(VirtualDir backend_) : backend{std::move(backend_)} {}
+    explicit IFileSystem(VirtualDir backend_) : backend{std::move(backend_)} {}
     virtual ~IFileSystem() {}
 
     Result CreateFile(const Path& path, s64 size, CreateOption option) {
@@ -158,8 +158,7 @@ private:
         R_RETURN(backend.OpenFile(out_file, path.GetString(), mode));
     }
 
-    Result DoOpenDirectory(VirtualDir* out_directory, const Path& path,
+    Result DoOpenDirectory(VirtualDir* out_directory, const Path& path, OpenDirectoryMode mode) {
-                           OpenDirectoryMode mode) {
         R_RETURN(backend.OpenDirectory(out_directory, path.GetString()));
     }
 

src/core/file_sys/fssrv/fssrv_sf_path.h

@@ -33,4 +33,4 @@ static_assert(std::is_trivially_copyable_v<Path>, "Path must be trivially copyab
 
 using FspPath = Path;
 
-} // namespace FileSys::Sf
+} // namespace FileSys::Sf

src/core/hle/service/filesystem/fsp/fs_i_filesystem.cpp

@@ -11,8 +11,8 @@
 namespace Service::FileSystem {
 
 IFileSystem::IFileSystem(Core::System& system_, FileSys::VirtualDir dir_, SizeGetter size_getter_)
-    : ServiceFramework{system_, "IFileSystem"},
+    : ServiceFramework{system_, "IFileSystem"}, backend{std::make_unique<FileSys::Fsa::IFileSystem>(
-      backend{std::make_unique<FileSys::Fsa::IFileSystem>(dir_)},
+                                                    dir_)},
       size_getter{std::move(size_getter_)} {
     static const FunctionInfo functions[] = {
         {0, D<&IFileSystem::CreateFile>, "CreateFile"},

src/core/hle/service/filesystem/fsp/fs_i_filesystem.h

@@ -3,6 +3,7 @@
 
 #pragma once
 
+#include "common/common_funcs.h"
 #include "core/file_sys/fsa/fs_i_filesystem.h"
 #include "core/file_sys/vfs/vfs.h"
 #include "core/hle/service/cmif_types.h"
@@ -48,8 +49,8 @@ public:
     };
     static_assert(sizeof(FileSystemAttribute) == 0xC0, "FileSystemAttribute has incorrect size");
 
-    Result CreateFile(const InLargeData<FileSys::Sf::Path, BufferAttr_HipcPointer> path,
+    Result CreateFile(const InLargeData<FileSys::Sf::Path, BufferAttr_HipcPointer> path, s32 option,
-                      s32 option, s64 size);
+                      s64 size);
     Result DeleteFile(const InLargeData<FileSys::Sf::Path, BufferAttr_HipcPointer> path);
     Result CreateDirectory(const InLargeData<FileSys::Sf::Path, BufferAttr_HipcPointer> path);
     Result DeleteDirectory(const InLargeData<FileSys::Sf::Path, BufferAttr_HipcPointer> path);