From ba70dc4c13ff84b51d2937f5c8ba873b061cb4c1 Mon Sep 17 00:00:00 2001 From: FearlessTobi Date: Sun, 11 Feb 2024 22:27:20 +0100 Subject: [PATCH] Address review comments --- src/core/file_sys/fs_filesystem.h | 2 +- src/core/file_sys/fs_path_utility.h | 15 +++++++++----- src/core/file_sys/fsa/fs_i_directory.h | 20 ++++++++++--------- src/core/file_sys/fsa/fs_i_file.h | 6 ++++-- src/core/file_sys/fsa/fs_i_filesystem.h | 7 +++---- src/core/file_sys/fssrv/fssrv_sf_path.h | 2 +- .../filesystem/fsp/fs_i_filesystem.cpp | 4 ++-- .../service/filesystem/fsp/fs_i_filesystem.h | 5 +++-- 8 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/core/file_sys/fs_filesystem.h b/src/core/file_sys/fs_filesystem.h index 7065ef6a6..598b59a74 100644 --- a/src/core/file_sys/fs_filesystem.h +++ b/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) diff --git a/src/core/file_sys/fs_path_utility.h b/src/core/file_sys/fs_path_utility.h index 5643141f9..51418ee16 100644 --- a/src/core/file_sys/fs_path_utility.h +++ b/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 void Allow##__WHICH__() { m_value |= __WHICH__##Flag; } + constexpr bool Is##__WHICH__##Allowed() const { \ + 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, - bool is_windows_path, bool is_drive_relative_path, - bool allow_all_characters = false) { + static constexpr Result Normalize(char* dst, size_t* out_len, const char* path, + size_t max_out_size, bool is_windows_path, + bool is_drive_relative_path, + bool allow_all_characters = false) { // Use StringTraits names for remainder of scope using namespace StringTraits; diff --git a/src/core/file_sys/fsa/fs_i_directory.h b/src/core/file_sys/fsa/fs_i_directory.h index a4135efec..fc0407d01 100644 --- a/src/core/file_sys/fsa/fs_i_directory.h +++ b/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(max_entries), entries.size() - next_entry_index); - auto* begin = reinterpret_cast(entries.data() + next_entry_index); + const auto* begin = reinterpret_cast(entries.data() + next_entry_index); + const auto* end = reinterpret_cast(entries.data() + next_entry_index + actual_entries); + const auto range_size = static_cast(std::distance(begin, end)); next_entry_index += actual_entries; *out_count = actual_entries; - out_entries = reinterpret_cast(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 - void BuildEntryIndex(std::vector& entries, const std::vector& new_data, - DirectoryEntryType type) { + void BuildEntryIndex(const std::vector& new_data, DirectoryEntryType type) { entries.reserve(entries.size() + new_data.size()); for (const auto& new_entry : new_data) { diff --git a/src/core/file_sys/fsa/fs_i_file.h b/src/core/file_sys/fsa/fs_i_file.h index 6dd0f6439..8fdd71c80 100644 --- a/src/core/file_sys/fsa/fs_i_file.h +++ b/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 output = backend->ReadBytes(size, offset); + *out = output.size(); - buffer = output.data(); + std::memcpy(buffer, output.data(), size); + R_SUCCEED(); } diff --git a/src/core/file_sys/fsa/fs_i_filesystem.h b/src/core/file_sys/fsa/fs_i_filesystem.h index e92284459..8172190f4 100644 --- a/src/core/file_sys/fsa/fs_i_filesystem.h +++ b/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, - OpenDirectoryMode mode) { + Result DoOpenDirectory(VirtualDir* out_directory, const Path& path, OpenDirectoryMode mode) { R_RETURN(backend.OpenDirectory(out_directory, path.GetString())); } diff --git a/src/core/file_sys/fssrv/fssrv_sf_path.h b/src/core/file_sys/fssrv/fssrv_sf_path.h index 1752a413d..a0c0b2dac 100644 --- a/src/core/file_sys/fssrv/fssrv_sf_path.h +++ b/src/core/file_sys/fssrv/fssrv_sf_path.h @@ -33,4 +33,4 @@ static_assert(std::is_trivially_copyable_v, "Path must be trivially copyab using FspPath = Path; -} // namespace FileSys::Sf \ No newline at end of file +} // namespace FileSys::Sf diff --git a/src/core/hle/service/filesystem/fsp/fs_i_filesystem.cpp b/src/core/hle/service/filesystem/fsp/fs_i_filesystem.cpp index 7fc62cb3e..86dd5b7e9 100644 --- a/src/core/hle/service/filesystem/fsp/fs_i_filesystem.cpp +++ b/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"}, - backend{std::make_unique(dir_)}, + : ServiceFramework{system_, "IFileSystem"}, backend{std::make_unique( + dir_)}, size_getter{std::move(size_getter_)} { static const FunctionInfo functions[] = { {0, D<&IFileSystem::CreateFile>, "CreateFile"}, diff --git a/src/core/hle/service/filesystem/fsp/fs_i_filesystem.h b/src/core/hle/service/filesystem/fsp/fs_i_filesystem.h index d07b74938..230ab8d71 100644 --- a/src/core/hle/service/filesystem/fsp/fs_i_filesystem.h +++ b/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 path, - s32 option, s64 size); + Result CreateFile(const InLargeData path, s32 option, + s64 size); Result DeleteFile(const InLargeData path); Result CreateDirectory(const InLargeData path); Result DeleteDirectory(const InLargeData path);