From ba70dc4c13ff84b51d2937f5c8ba873b061cb4c1 Mon Sep 17 00:00:00 2001
From: FearlessTobi <thm.frey@gmail.com>
Date: Sun, 11 Feb 2024 22:27:20 +0100
Subject: 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 +-
.../hle/service/filesystem/fsp/fs_i_filesystem.cpp | 4 ++--
.../hle/service/filesystem/fsp/fs_i_filesystem.h | 5 +++--
8 files changed, 35 insertions(+), 26 deletions(-)
(limited to 'src')
diff --git a/src/core/file_sys/fs_filesystem.h b/src/core/file_sys/fs_filesystem.h
index 7065ef6a69..598b59a747 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 5643141f9f..51418ee160 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 a4135efec0..fc0407d013 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<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,
- DirectoryEntryType type) {
+ void BuildEntryIndex(const std::vector<T>& 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 6dd0f64393..8fdd71c80f 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<u8> 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 e92284459b..8172190f49 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 1752a413dd..a0c0b2dac8 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>, "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 7fc62cb3e4..86dd5b7e97 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<FileSys::Fsa::IFileSystem>(dir_)},
+ : ServiceFramework{system_, "IFileSystem"}, backend{std::make_unique<FileSys::Fsa::IFileSystem>(
+ 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 d07b74938c..230ab8d71d 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<FileSys::Sf::Path, BufferAttr_HipcPointer> path,
- s32 option, s64 size);
+ Result CreateFile(const InLargeData<FileSys::Sf::Path, BufferAttr_HipcPointer> path, s32 option,
+ 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);
--
cgit v1.2.3-70-g09d2