From 39ae73b356a253036283b114855f8c5ddbb20f49 Mon Sep 17 00:00:00 2001
From: Lioncash <mathew1800@gmail.com>
Date: Tue, 16 Oct 2018 09:05:47 -0400
Subject: [PATCH] file_sys/registered_cache: Use unique_ptr and regular
pointers instead of shared_ptrs where applicable
The data retrieved in these cases are ultimately chiefly owned by either
the RegisteredCache instance itself, or the filesystem factories. Both
these should live throughout the use of their contained data. If they
don't, it should be considered an interface/design issue, and using
shared_ptr instances here would mask that, as the data would always be
prolonged after the main owner's lifetime ended.
This makes the lifetime of the data explicit and makes it harder to
accidentally create cyclic references. It also makes the interface
slightly more flexible than the previous API, as a shared_ptr can be
created from a unique_ptr, but not the other way around, so this allows
for that use-case if it ever becomes necessary in some form.
---
src/core/file_sys/bis_factory.cpp | 12 ++++++------
src/core/file_sys/bis_factory.h | 8 ++++----
src/core/file_sys/patch_manager.cpp | 2 +-
src/core/file_sys/registered_cache.cpp | 14 +++++++-------
src/core/file_sys/registered_cache.h | 12 ++++++------
src/core/file_sys/sdmc_factory.cpp | 8 ++++----
src/core/file_sys/sdmc_factory.h | 4 ++--
.../hle/service/filesystem/filesystem.cpp | 13 ++++++-------
src/core/hle/service/filesystem/filesystem.h | 8 ++++----
src/core/hle/service/ns/pl_u.cpp | 2 +-
src/yuzu/game_list_worker.cpp | 19 +++++++++++--------
src/yuzu/game_list_worker.h | 2 +-
12 files changed, 53 insertions(+), 51 deletions(-)
diff --git a/src/core/file_sys/bis_factory.cpp b/src/core/file_sys/bis_factory.cpp
index 6102ef476e..76a2b7e863 100644
--- a/src/core/file_sys/bis_factory.cpp
+++ b/src/core/file_sys/bis_factory.cpp
@@ -10,19 +10,19 @@ namespace FileSys {
BISFactory::BISFactory(VirtualDir nand_root_, VirtualDir load_root_)
: nand_root(std::move(nand_root_)), load_root(std::move(load_root_)),
- sysnand_cache(std::make_shared<RegisteredCache>(
+ sysnand_cache(std::make_unique<RegisteredCache>(
GetOrCreateDirectoryRelative(nand_root, "/system/Contents/registered"))),
- usrnand_cache(std::make_shared<RegisteredCache>(
+ usrnand_cache(std::make_unique<RegisteredCache>(
GetOrCreateDirectoryRelative(nand_root, "/user/Contents/registered"))) {}
BISFactory::~BISFactory() = default;
-std::shared_ptr<RegisteredCache> BISFactory::GetSystemNANDContents() const {
- return sysnand_cache;
+RegisteredCache* BISFactory::GetSystemNANDContents() const {
+ return sysnand_cache.get();
}
-std::shared_ptr<RegisteredCache> BISFactory::GetUserNANDContents() const {
- return usrnand_cache;
+RegisteredCache* BISFactory::GetUserNANDContents() const {
+ return usrnand_cache.get();
}
VirtualDir BISFactory::GetModificationLoadRoot(u64 title_id) const {
diff --git a/src/core/file_sys/bis_factory.h b/src/core/file_sys/bis_factory.h
index c352e09256..364d309bd2 100644
--- a/src/core/file_sys/bis_factory.h
+++ b/src/core/file_sys/bis_factory.h
@@ -20,8 +20,8 @@ public:
explicit BISFactory(VirtualDir nand_root, VirtualDir load_root);
~BISFactory();
- std::shared_ptr<RegisteredCache> GetSystemNANDContents() const;
- std::shared_ptr<RegisteredCache> GetUserNANDContents() const;
+ RegisteredCache* GetSystemNANDContents() const;
+ RegisteredCache* GetUserNANDContents() const;
VirtualDir GetModificationLoadRoot(u64 title_id) const;
@@ -29,8 +29,8 @@ private:
VirtualDir nand_root;
VirtualDir load_root;
- std::shared_ptr<RegisteredCache> sysnand_cache;
- std::shared_ptr<RegisteredCache> usrnand_cache;
+ std::unique_ptr<RegisteredCache> sysnand_cache;
+ std::unique_ptr<RegisteredCache> usrnand_cache;
};
} // namespace FileSys
diff --git a/src/core/file_sys/patch_manager.cpp b/src/core/file_sys/patch_manager.cpp
index 019caebe93..c15ac8e19f 100644
--- a/src/core/file_sys/patch_manager.cpp
+++ b/src/core/file_sys/patch_manager.cpp
@@ -346,7 +346,7 @@ std::map<std::string, std::string, std::less<>> PatchManager::GetPatchVersionNam
}
std::pair<std::unique_ptr<NACP>, VirtualFile> PatchManager::GetControlMetadata() const {
- const auto& installed{Service::FileSystem::GetUnionContents()};
+ const auto installed{Service::FileSystem::GetUnionContents()};
const auto base_control_nca = installed->GetEntry(title_id, ContentRecordType::Control);
if (base_control_nca == nullptr)
diff --git a/src/core/file_sys/registered_cache.cpp b/src/core/file_sys/registered_cache.cpp
index e9b0406894..1febb398e1 100644
--- a/src/core/file_sys/registered_cache.cpp
+++ b/src/core/file_sys/registered_cache.cpp
@@ -308,14 +308,14 @@ VirtualFile RegisteredCache::GetEntryRaw(RegisteredCacheEntry entry) const {
return GetEntryRaw(entry.title_id, entry.type);
}
-std::shared_ptr<NCA> RegisteredCache::GetEntry(u64 title_id, ContentRecordType type) const {
+std::unique_ptr<NCA> RegisteredCache::GetEntry(u64 title_id, ContentRecordType type) const {
const auto raw = GetEntryRaw(title_id, type);
if (raw == nullptr)
return nullptr;
- return std::make_shared<NCA>(raw);
+ return std::make_unique<NCA>(raw);
}
-std::shared_ptr<NCA> RegisteredCache::GetEntry(RegisteredCacheEntry entry) const {
+std::unique_ptr<NCA> RegisteredCache::GetEntry(RegisteredCacheEntry entry) const {
return GetEntry(entry.title_id, entry.type);
}
@@ -516,7 +516,7 @@ bool RegisteredCache::RawInstallYuzuMeta(const CNMT& cnmt) {
}) != yuzu_meta.end();
}
-RegisteredCacheUnion::RegisteredCacheUnion(std::vector<std::shared_ptr<RegisteredCache>> caches)
+RegisteredCacheUnion::RegisteredCacheUnion(std::vector<RegisteredCache*> caches)
: caches(std::move(caches)) {}
void RegisteredCacheUnion::Refresh() {
@@ -572,14 +572,14 @@ VirtualFile RegisteredCacheUnion::GetEntryRaw(RegisteredCacheEntry entry) const
return GetEntryRaw(entry.title_id, entry.type);
}
-std::shared_ptr<NCA> RegisteredCacheUnion::GetEntry(u64 title_id, ContentRecordType type) const {
+std::unique_ptr<NCA> RegisteredCacheUnion::GetEntry(u64 title_id, ContentRecordType type) const {
const auto raw = GetEntryRaw(title_id, type);
if (raw == nullptr)
return nullptr;
- return std::make_shared<NCA>(raw);
+ return std::make_unique<NCA>(raw);
}
-std::shared_ptr<NCA> RegisteredCacheUnion::GetEntry(RegisteredCacheEntry entry) const {
+std::unique_ptr<NCA> RegisteredCacheUnion::GetEntry(RegisteredCacheEntry entry) const {
return GetEntry(entry.title_id, entry.type);
}
diff --git a/src/core/file_sys/registered_cache.h b/src/core/file_sys/registered_cache.h
index c0cd59fc57..5ddacba475 100644
--- a/src/core/file_sys/registered_cache.h
+++ b/src/core/file_sys/registered_cache.h
@@ -88,8 +88,8 @@ public:
VirtualFile GetEntryRaw(u64 title_id, ContentRecordType type) const;
VirtualFile GetEntryRaw(RegisteredCacheEntry entry) const;
- std::shared_ptr<NCA> GetEntry(u64 title_id, ContentRecordType type) const;
- std::shared_ptr<NCA> GetEntry(RegisteredCacheEntry entry) const;
+ std::unique_ptr<NCA> GetEntry(u64 title_id, ContentRecordType type) const;
+ std::unique_ptr<NCA> GetEntry(RegisteredCacheEntry entry) const;
std::vector<RegisteredCacheEntry> ListEntries() const;
// If a parameter is not boost::none, it will be filtered for from all entries.
@@ -142,7 +142,7 @@ private:
// Combines multiple RegisteredCaches (i.e. SysNAND, UserNAND, SDMC) into one interface.
class RegisteredCacheUnion {
public:
- explicit RegisteredCacheUnion(std::vector<std::shared_ptr<RegisteredCache>> caches);
+ explicit RegisteredCacheUnion(std::vector<RegisteredCache*> caches);
void Refresh();
@@ -157,8 +157,8 @@ public:
VirtualFile GetEntryRaw(u64 title_id, ContentRecordType type) const;
VirtualFile GetEntryRaw(RegisteredCacheEntry entry) const;
- std::shared_ptr<NCA> GetEntry(u64 title_id, ContentRecordType type) const;
- std::shared_ptr<NCA> GetEntry(RegisteredCacheEntry entry) const;
+ std::unique_ptr<NCA> GetEntry(u64 title_id, ContentRecordType type) const;
+ std::unique_ptr<NCA> GetEntry(RegisteredCacheEntry entry) const;
std::vector<RegisteredCacheEntry> ListEntries() const;
// If a parameter is not boost::none, it will be filtered for from all entries.
@@ -168,7 +168,7 @@ public:
boost::optional<u64> title_id = boost::none) const;
private:
- std::vector<std::shared_ptr<RegisteredCache>> caches;
+ std::vector<RegisteredCache*> caches;
};
} // namespace FileSys
diff --git a/src/core/file_sys/sdmc_factory.cpp b/src/core/file_sys/sdmc_factory.cpp
index d66a9c9a42..bd3a570588 100644
--- a/src/core/file_sys/sdmc_factory.cpp
+++ b/src/core/file_sys/sdmc_factory.cpp
@@ -10,10 +10,10 @@
namespace FileSys {
SDMCFactory::SDMCFactory(VirtualDir dir_)
- : dir(std::move(dir_)), contents(std::make_shared<RegisteredCache>(
+ : dir(std::move(dir_)), contents(std::make_unique<RegisteredCache>(
GetOrCreateDirectoryRelative(dir, "/Nintendo/Contents/registered"),
[](const VirtualFile& file, const NcaID& id) {
- return std::make_shared<NAX>(file, id)->GetDecrypted();
+ return NAX{file, id}.GetDecrypted();
})) {}
SDMCFactory::~SDMCFactory() = default;
@@ -22,8 +22,8 @@ ResultVal<VirtualDir> SDMCFactory::Open() {
return MakeResult<VirtualDir>(dir);
}
-std::shared_ptr<RegisteredCache> SDMCFactory::GetSDMCContents() const {
- return contents;
+RegisteredCache* SDMCFactory::GetSDMCContents() const {
+ return contents.get();
}
} // namespace FileSys
diff --git a/src/core/file_sys/sdmc_factory.h b/src/core/file_sys/sdmc_factory.h
index ea12149def..42794ba5b4 100644
--- a/src/core/file_sys/sdmc_factory.h
+++ b/src/core/file_sys/sdmc_factory.h
@@ -19,12 +19,12 @@ public:
~SDMCFactory();
ResultVal<VirtualDir> Open();
- std::shared_ptr<RegisteredCache> GetSDMCContents() const;
+ RegisteredCache* GetSDMCContents() const;
private:
VirtualDir dir;
- std::shared_ptr<RegisteredCache> contents;
+ std::unique_ptr<RegisteredCache> contents;
};
} // namespace FileSys
diff --git a/src/core/hle/service/filesystem/filesystem.cpp b/src/core/hle/service/filesystem/filesystem.cpp
index e06712603b..e32a7c48e2 100644
--- a/src/core/hle/service/filesystem/filesystem.cpp
+++ b/src/core/hle/service/filesystem/filesystem.cpp
@@ -319,13 +319,12 @@ ResultVal<FileSys::VirtualDir> OpenSDMC() {
return sdmc_factory->Open();
}
-std::shared_ptr<FileSys::RegisteredCacheUnion> GetUnionContents() {
- return std::make_shared<FileSys::RegisteredCacheUnion>(
- std::vector<std::shared_ptr<FileSys::RegisteredCache>>{
- GetSystemNANDContents(), GetUserNANDContents(), GetSDMCContents()});
+std::unique_ptr<FileSys::RegisteredCacheUnion> GetUnionContents() {
+ return std::make_unique<FileSys::RegisteredCacheUnion>(std::vector<FileSys::RegisteredCache*>{
+ GetSystemNANDContents(), GetUserNANDContents(), GetSDMCContents()});
}
-std::shared_ptr<FileSys::RegisteredCache> GetSystemNANDContents() {
+FileSys::RegisteredCache* GetSystemNANDContents() {
LOG_TRACE(Service_FS, "Opening System NAND Contents");
if (bis_factory == nullptr)
@@ -334,7 +333,7 @@ std::shared_ptr<FileSys::RegisteredCache> GetSystemNANDContents() {
return bis_factory->GetSystemNANDContents();
}
-std::shared_ptr<FileSys::RegisteredCache> GetUserNANDContents() {
+FileSys::RegisteredCache* GetUserNANDContents() {
LOG_TRACE(Service_FS, "Opening User NAND Contents");
if (bis_factory == nullptr)
@@ -343,7 +342,7 @@ std::shared_ptr<FileSys::RegisteredCache> GetUserNANDContents() {
return bis_factory->GetUserNANDContents();
}
-std::shared_ptr<FileSys::RegisteredCache> GetSDMCContents() {
+FileSys::RegisteredCache* GetSDMCContents() {
LOG_TRACE(Service_FS, "Opening SDMC Contents");
if (sdmc_factory == nullptr)
diff --git a/src/core/hle/service/filesystem/filesystem.h b/src/core/hle/service/filesystem/filesystem.h
index 2df1faeb07..6ca5c56369 100644
--- a/src/core/hle/service/filesystem/filesystem.h
+++ b/src/core/hle/service/filesystem/filesystem.h
@@ -47,11 +47,11 @@ ResultVal<FileSys::VirtualDir> OpenSaveData(FileSys::SaveDataSpaceId space,
FileSys::SaveDataDescriptor save_struct);
ResultVal<FileSys::VirtualDir> OpenSDMC();
-std::shared_ptr<FileSys::RegisteredCacheUnion> GetUnionContents();
+std::unique_ptr<FileSys::RegisteredCacheUnion> GetUnionContents();
-std::shared_ptr<FileSys::RegisteredCache> GetSystemNANDContents();
-std::shared_ptr<FileSys::RegisteredCache> GetUserNANDContents();
-std::shared_ptr<FileSys::RegisteredCache> GetSDMCContents();
+FileSys::RegisteredCache* GetSystemNANDContents();
+FileSys::RegisteredCache* GetUserNANDContents();
+FileSys::RegisteredCache* GetSDMCContents();
FileSys::VirtualDir GetModificationLoadRoot(u64 title_id);
diff --git a/src/core/hle/service/ns/pl_u.cpp b/src/core/hle/service/ns/pl_u.cpp
index 4b2f758a88..44accecb7d 100644
--- a/src/core/hle/service/ns/pl_u.cpp
+++ b/src/core/hle/service/ns/pl_u.cpp
@@ -161,7 +161,7 @@ PL_U::PL_U() : ServiceFramework("pl:u"), impl{std::make_unique<Impl>()} {
};
RegisterHandlers(functions);
// Attempt to load shared font data from disk
- const auto nand = FileSystem::GetSystemNANDContents();
+ const auto* nand = FileSystem::GetSystemNANDContents();
std::size_t offset = 0;
// Rebuild shared fonts from data ncas
if (nand->HasEntry(static_cast<u64>(FontArchives::Standard),
diff --git a/src/yuzu/game_list_worker.cpp b/src/yuzu/game_list_worker.cpp
index 8f99a1c789..3881aba5f0 100644
--- a/src/yuzu/game_list_worker.cpp
+++ b/src/yuzu/game_list_worker.cpp
@@ -96,7 +96,7 @@ void GameListWorker::AddInstalledTitlesToGameList() {
FileSys::ContentRecordType::Program);
for (const auto& game : installed_games) {
- const auto& file = cache->GetEntryUnparsed(game);
+ const auto file = cache->GetEntryUnparsed(game);
std::unique_ptr<Loader::AppLoader> loader = Loader::GetLoader(file);
if (!loader)
continue;
@@ -107,7 +107,7 @@ void GameListWorker::AddInstalledTitlesToGameList() {
loader->ReadProgramId(program_id);
const FileSys::PatchManager patch{program_id};
- const auto& control = cache->GetEntry(game.title_id, FileSys::ContentRecordType::Control);
+ const auto control = cache->GetEntry(game.title_id, FileSys::ContentRecordType::Control);
if (control != nullptr)
GetMetadataFromControlNCA(patch, *control, icon, name);
@@ -135,9 +135,10 @@ void GameListWorker::AddInstalledTitlesToGameList() {
FileSys::ContentRecordType::Control);
for (const auto& entry : control_data) {
- const auto nca = cache->GetEntry(entry);
- if (nca != nullptr)
- nca_control_map.insert_or_assign(entry.title_id, nca);
+ auto nca = cache->GetEntry(entry);
+ if (nca != nullptr) {
+ nca_control_map.insert_or_assign(entry.title_id, std::move(nca));
+ }
}
}
@@ -153,9 +154,11 @@ void GameListWorker::FillControlMap(const std::string& dir_path) {
QFileInfo file_info(physical_name.c_str());
if (!is_dir && file_info.suffix().toStdString() == "nca") {
auto nca =
- std::make_shared<FileSys::NCA>(vfs->OpenFile(physical_name, FileSys::Mode::Read));
- if (nca->GetType() == FileSys::NCAContentType::Control)
- nca_control_map.insert_or_assign(nca->GetTitleId(), nca);
+ std::make_unique<FileSys::NCA>(vfs->OpenFile(physical_name, FileSys::Mode::Read));
+ if (nca->GetType() == FileSys::NCAContentType::Control) {
+ const u64 title_id = nca->GetTitleId();
+ nca_control_map.insert_or_assign(title_id, std::move(nca));
+ }
}
return true;
};
diff --git a/src/yuzu/game_list_worker.h b/src/yuzu/game_list_worker.h
index 09d20c42fc..0e42d0bded 100644
--- a/src/yuzu/game_list_worker.h
+++ b/src/yuzu/game_list_worker.h
@@ -63,7 +63,7 @@ private:
void AddFstEntriesToGameList(const std::string& dir_path, unsigned int recursion = 0);
std::shared_ptr<FileSys::VfsFilesystem> vfs;
- std::map<u64, std::shared_ptr<FileSys::NCA>> nca_control_map;
+ std::map<u64, std::unique_ptr<FileSys::NCA>> nca_control_map;
QStringList watch_list;
QString dir_path;
bool deep_scan;
--
GitLab