From 1b356cd4f21b35690a2e884a5289ca3a982db7d2 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 19 Jun 2018 18:07:31 +0000 Subject: [PATCH] Modernize C++ usage (#144) * Stratosphere: Use modern C++ idioms in some places * algorithms like std::for_each are used instead of raw loops * Stratosphere: Replace more raw loops with algorithms * Stratosphere: Add a utility predicate function to test for equality with a reference element This can be used to rewrite some common raw loops using algorithms instead * fs.mitm: Use variant * fs.mitm: Use enum class * fs.mitm: Turn RomFSSourceInfo::Cleanup into a destructor This obsoletes the need for a custom deleter in other places * fs.mitm: Use enum class some more * fs.mitm: Use unique_ptr * fs.mitm: Simplify initialization * Stratosphere: Simplify initialization * fs.mitm: Use unique_ptr (fix memory leak along the way) The previous code was using "delete" rather than "delete[]" * fs.mitm: Use vector::emplace_back rather than push_back emplace_back constructs elements in-place, hence avoiding a redundant element copy. * Stratosphere: Replace more raw loops with algorithms * Stratosphere: Use unique_ptr * fs.mitm: Replace more raw loops with algorithms * Stratosphere: Prefer move-construction over copy-construction when moving sink parameters around --- include/meta_tools.hpp | 37 +++++++++++++ include/stratosphere/domainowner.hpp | 58 ++++++++------------ include/stratosphere/ievent.hpp | 7 +-- include/stratosphere/ipcsession.hpp | 8 +-- include/stratosphere/iserver.hpp | 3 +- include/stratosphere/isession.hpp | 41 ++++---------- include/stratosphere/waitablemanager.hpp | 15 +++-- include/stratosphere/waitablemanagerbase.hpp | 12 ++-- source/multithreadedwaitablemanager.cpp | 15 ++--- source/waitablemanager.cpp | 15 ++--- 10 files changed, 102 insertions(+), 109 deletions(-) create mode 100644 include/meta_tools.hpp diff --git a/include/meta_tools.hpp b/include/meta_tools.hpp new file mode 100644 index 00000000..27da96a5 --- /dev/null +++ b/include/meta_tools.hpp @@ -0,0 +1,37 @@ +#pragma once + +#include + +namespace detail { + +template +struct class_of; + +template +struct class_of { + using type = C; +}; + +template +using class_of_t = typename class_of::type; + +template> +struct member_equals_fn_helper { + T ref; + Mem mem_fn; + + bool operator()(const C& val) const { + return (std::mem_fn(mem_fn)(val) == ref); + } + + bool operator()(C&& val) const { + return (std::mem_fn(mem_fn)(std::move(val)) == ref); + } +}; + +} // namespace detail + +template +auto member_equals_fn(Mem mem, T ref) { + return detail::member_equals_fn_helper{std::move(ref), std::move(mem)}; +} diff --git a/include/stratosphere/domainowner.hpp b/include/stratosphere/domainowner.hpp index 09dde791..adb64b9c 100644 --- a/include/stratosphere/domainowner.hpp +++ b/include/stratosphere/domainowner.hpp @@ -1,5 +1,6 @@ #pragma once #include +#include #include #include @@ -11,18 +12,13 @@ class IServiceObject; class DomainOwner { private: - std::shared_ptr domain_objects[DOMAIN_ID_MAX]; + std::array, DOMAIN_ID_MAX> domain_objects; public: - DomainOwner() { - for (unsigned int i = 0; i < DOMAIN_ID_MAX; i++) { - domain_objects[i].reset(); - } - } - - virtual ~DomainOwner() { - /* Shared ptrs should auto delete here. */ - } - + DomainOwner() = default; + + /* Shared ptrs should auto delete here. */ + virtual ~DomainOwner() = default; + std::shared_ptr get_domain_object(unsigned int i) { if (i < DOMAIN_ID_MAX) { return domain_objects[i]; @@ -31,20 +27,20 @@ class DomainOwner { } Result reserve_object(std::shared_ptr object, unsigned int *out_i) { - for (unsigned int i = 4; i < DOMAIN_ID_MAX; i++) { - if (domain_objects[i] == NULL) { - domain_objects[i] = object; - object->set_owner(this); - *out_i = i; - return 0; - } + auto object_it = std::find(domain_objects.begin() + 4, domain_objects.end(), nullptr); + if (object_it == domain_objects.end()) { + return 0x1900B; } - return 0x1900B; + + *out_i = std::distance(domain_objects.begin(), object_it); + *object_it = std::move(object); + (*object_it)->set_owner(this); + return 0; } Result set_object(std::shared_ptr object, unsigned int i) { if (domain_objects[i] == NULL) { - domain_objects[i] = object; + domain_objects[i] = std::move(object); object->set_owner(this); return 0; } @@ -52,26 +48,18 @@ class DomainOwner { } unsigned int get_object_id(std::shared_ptr object) { - for (unsigned int i = 0; i < DOMAIN_ID_MAX; i++) { - if (domain_objects[i] == object) { - return i; - } - } - return DOMAIN_ID_MAX; + auto object_it = std::find(domain_objects.begin(), domain_objects.end(), object); + return std::distance(domain_objects.begin(), object_it); } void delete_object(unsigned int i) { - if (domain_objects[i]) { - domain_objects[i].reset(); - } + domain_objects[i].reset(); } void delete_object(std::shared_ptr object) { - for (unsigned int i = 0; i < DOMAIN_ID_MAX; i++) { - if (domain_objects[i] == object) { - domain_objects[i].reset(); - break; - } + auto object_it = std::find(domain_objects.begin(), domain_objects.end(), object); + if (object_it != domain_objects.end()) { + object_it->reset(); } } -}; \ No newline at end of file +}; diff --git a/include/stratosphere/ievent.hpp b/include/stratosphere/ievent.hpp index 4d5b1582..cdff65c1 100644 --- a/include/stratosphere/ievent.hpp +++ b/include/stratosphere/ievent.hpp @@ -1,5 +1,6 @@ #pragma once #include +#include #include #include "iwaitable.hpp" @@ -22,9 +23,7 @@ class IEvent : public IWaitable { } ~IEvent() { - for (auto &h : this->handles) { - svcCloseHandle(h); - } + std::for_each(handles.begin(), handles.end(), svcCloseHandle); } virtual Result signal_event() = 0; @@ -50,4 +49,4 @@ class IEvent : public IWaitable { /* TODO: Panic. */ return 0xCAFE; } -}; \ No newline at end of file +}; diff --git a/include/stratosphere/ipcsession.hpp b/include/stratosphere/ipcsession.hpp index e3967723..9983a153 100644 --- a/include/stratosphere/ipcsession.hpp +++ b/include/stratosphere/ipcsession.hpp @@ -18,9 +18,7 @@ class IPCSession final : public ISession { fatalSimple(rc); } this->service_object = std::make_shared(); - this->pointer_buffer_size = pbs; - this->pointer_buffer = new char[this->pointer_buffer_size]; - this->is_domain = false; + this->pointer_buffer.resize(pbs); } IPCSession(std::shared_ptr so, size_t pbs = 0x400) : ISession(NULL, 0, 0, so, 0) { @@ -28,8 +26,6 @@ class IPCSession final : public ISession { if (R_FAILED((rc = svcCreateSession(&this->server_handle, &this->client_handle, 0, 0)))) { fatalSimple(rc); } - this->pointer_buffer_size = pbs; - this->pointer_buffer = new char[this->pointer_buffer_size]; - this->is_domain = false; + this->pointer_buffer.resize(pbs); } }; diff --git a/include/stratosphere/iserver.hpp b/include/stratosphere/iserver.hpp index 24ff1ee6..06d14040 100644 --- a/include/stratosphere/iserver.hpp +++ b/include/stratosphere/iserver.hpp @@ -1,5 +1,6 @@ #pragma once #include +#include #include #include "iserviceobject.hpp" @@ -51,4 +52,4 @@ class IServer : public IWaitable { this->get_manager()->add_waitable(this->get_new_session(session_h)); return 0; } -}; \ No newline at end of file +}; diff --git a/include/stratosphere/isession.hpp b/include/stratosphere/isession.hpp index 52df6c42..c0e44d0b 100644 --- a/include/stratosphere/isession.hpp +++ b/include/stratosphere/isession.hpp @@ -17,7 +17,6 @@ enum IpcControlCommand { IpcCtrl_Cmd_CloneCurrentObjectEx = 4 }; -#define POINTER_BUFFER_SIZE_MAX 0xFFFF #define RESULT_DEFER_SESSION (0x6580A) @@ -34,39 +33,23 @@ class ISession : public IWaitable { IServer *server; Handle server_handle; Handle client_handle; - char *pointer_buffer; - size_t pointer_buffer_size; + std::vector pointer_buffer; - bool is_domain; + bool is_domain = false; std::shared_ptr domain; std::shared_ptr active_object; - static_assert(sizeof(pointer_buffer) <= POINTER_BUFFER_SIZE_MAX, "Incorrect Size for PointerBuffer!"); - public: - ISession(IServer *s, Handle s_h, Handle c_h, size_t pbs = 0x400) : server(s), server_handle(s_h), client_handle(c_h), pointer_buffer_size(pbs) { + ISession(IServer *s, Handle s_h, Handle c_h, size_t pbs = 0x400) : server(s), server_handle(s_h), client_handle(c_h), pointer_buffer(pbs) { this->service_object = std::make_shared(); - if (this->pointer_buffer_size) { - this->pointer_buffer = new char[this->pointer_buffer_size]; - } - this->is_domain = false; - this->domain.reset(); - this->active_object.reset(); } - ISession(IServer *s, Handle s_h, Handle c_h, std::shared_ptr so, size_t pbs = 0x400) : service_object(so), server(s), server_handle(s_h), client_handle(c_h), pointer_buffer_size(pbs) { - if (this->pointer_buffer_size) { - this->pointer_buffer = new char[this->pointer_buffer_size]; - } - this->is_domain = false; - this->domain.reset(); - this->active_object.reset(); + ISession(IServer *s, Handle s_h, Handle c_h, std::shared_ptr so, size_t pbs = 0x400) : service_object(so), server(s), server_handle(s_h), client_handle(c_h), pointer_buffer(pbs) { } ~ISession() override { - delete this->pointer_buffer; if (server_handle) { svcCloseHandle(server_handle); } @@ -153,7 +136,7 @@ class ISession : public IWaitable { break; case IpcCommandType_Request: case IpcCommandType_RequestWithContext: - retval = this->active_object->dispatch(r, c, cmd_id, (u8 *)this->pointer_buffer, this->pointer_buffer_size); + retval = this->active_object->dispatch(r, c, cmd_id, (u8 *)pointer_buffer.data(), pointer_buffer.size()); break; case IpcCommandType_Control: case IpcCommandType_ControlWithContext: @@ -185,7 +168,7 @@ class ISession : public IWaitable { /* Prepare pointer buffer... */ IpcCommand c_for_reply; ipcInitialize(&c_for_reply); - ipcAddRecvStatic(&c_for_reply, this->pointer_buffer, this->pointer_buffer_size, 0); + ipcAddRecvStatic(&c_for_reply, this->pointer_buffer.data(), this->pointer_buffer.size(), 0); ipcPrepareHeader(&c_for_reply, 0); if (R_SUCCEEDED(rc = svcReplyAndReceive(&handle_index, &this->server_handle, 1, 0, U64_MAX))) { @@ -247,19 +230,19 @@ class ISession : public IWaitable { /* TODO: Implement. */ switch ((IpcControlCommand)cmd_id) { case IpcCtrl_Cmd_ConvertCurrentObjectToDomain: - rc = WrapIpcCommandImpl<&ISession::ConvertCurrentObjectToDomain>(this, r, out_c, (u8 *)this->pointer_buffer, this->pointer_buffer_size); + rc = WrapIpcCommandImpl<&ISession::ConvertCurrentObjectToDomain>(this, r, out_c, (u8 *)this->pointer_buffer.data(), pointer_buffer.size()); break; case IpcCtrl_Cmd_CopyFromCurrentDomain: - rc = WrapIpcCommandImpl<&ISession::CopyFromCurrentDomain>(this, r, out_c, (u8 *)this->pointer_buffer, this->pointer_buffer_size); + rc = WrapIpcCommandImpl<&ISession::CopyFromCurrentDomain>(this, r, out_c, (u8 *)this->pointer_buffer.data(), pointer_buffer.size()); break; case IpcCtrl_Cmd_CloneCurrentObject: - rc = WrapIpcCommandImpl<&ISession::CloneCurrentObject>(this, r, out_c, (u8 *)this->pointer_buffer, this->pointer_buffer_size); + rc = WrapIpcCommandImpl<&ISession::CloneCurrentObject>(this, r, out_c, (u8 *)this->pointer_buffer.data(), pointer_buffer.size()); break; case IpcCtrl_Cmd_QueryPointerBufferSize: - rc = WrapIpcCommandImpl<&ISession::QueryPointerBufferSize>(this, r, out_c, (u8 *)this->pointer_buffer, this->pointer_buffer_size); + rc = WrapIpcCommandImpl<&ISession::QueryPointerBufferSize>(this, r, out_c, (u8 *)this->pointer_buffer.data(), pointer_buffer.size()); break; case IpcCtrl_Cmd_CloneCurrentObjectEx: - rc = WrapIpcCommandImpl<&ISession::CloneCurrentObjectEx>(this, r, out_c, (u8 *)this->pointer_buffer, sizeof(this->pointer_buffer)); + rc = WrapIpcCommandImpl<&ISession::CloneCurrentObjectEx>(this, r, out_c, (u8 *)this->pointer_buffer.data(), pointer_buffer.size()); break; default: break; @@ -282,7 +265,7 @@ class ISession : public IWaitable { return {0xF601}; } std::tuple QueryPointerBufferSize() { - return {0x0, (u32)this->pointer_buffer_size}; + return {0x0, (u32)this->pointer_buffer.size()}; } std::tuple CloneCurrentObjectEx() { /* TODO */ diff --git a/include/stratosphere/waitablemanager.hpp b/include/stratosphere/waitablemanager.hpp index 5c0006e4..4b42cd2c 100644 --- a/include/stratosphere/waitablemanager.hpp +++ b/include/stratosphere/waitablemanager.hpp @@ -1,5 +1,7 @@ #pragma once #include +#include +#include #include #include "waitablemanagerbase.hpp" @@ -12,22 +14,19 @@ class WaitableManager : public WaitableManagerBase { protected: std::vector to_add_waitables; std::vector waitables; - u64 timeout; + u64 timeout = 0; HosMutex lock; - std::atomic_bool has_new_items; + std::atomic_bool has_new_items = false; private: void process_internal(bool break_on_timeout); public: - WaitableManager(u64 t) : waitables(0), timeout(t), has_new_items(false) { } + WaitableManager(u64 t) : timeout(t) { } ~WaitableManager() override { /* This should call the destructor for every waitable. */ - for (auto & waitable : waitables) { - delete waitable; - } - waitables.clear(); + std::for_each(waitables.begin(), waitables.end(), std::default_delete{}); } virtual void add_waitable(IWaitable *waitable); virtual void process(); virtual void process_until_timeout(); -}; \ No newline at end of file +}; diff --git a/include/stratosphere/waitablemanagerbase.hpp b/include/stratosphere/waitablemanagerbase.hpp index 97271006..ed4c7775 100644 --- a/include/stratosphere/waitablemanagerbase.hpp +++ b/include/stratosphere/waitablemanagerbase.hpp @@ -4,12 +4,12 @@ #include class WaitableManagerBase { - std::atomic cur_priority; + std::atomic cur_priority = 0; public: - WaitableManagerBase() : cur_priority(0) { } - virtual ~WaitableManagerBase() { } - - u64 get_priority() { + WaitableManagerBase() = default; + virtual ~WaitableManagerBase() = default; + + u64 get_priority() { return std::atomic_fetch_add(&cur_priority, (u64)1); } -}; \ No newline at end of file +}; diff --git a/source/multithreadedwaitablemanager.cpp b/source/multithreadedwaitablemanager.cpp index 6acc53a6..dcf1a931 100644 --- a/source/multithreadedwaitablemanager.cpp +++ b/source/multithreadedwaitablemanager.cpp @@ -1,6 +1,7 @@ #include #include +#include #include @@ -44,21 +45,15 @@ IWaitable *MultiThreadedWaitableManager::get_waitable() { rc = svcWaitSynchronization(&handle_index, handles.data(), this->waitables.size(), this->timeout); IWaitable *w = this->waitables[handle_index]; if (R_SUCCEEDED(rc)) { - for (int i = 0; i < handle_index; i++) { - this->waitables[i]->update_priority(); - } + std::for_each(waitables.begin(), waitables.begin() + handle_index, std::mem_fn(&IWaitable::update_priority)); this->waitables.erase(this->waitables.begin() + handle_index); } else if (rc == 0xEA01) { /* Timeout. */ - for (auto & waitable : this->waitables) { - waitable->update_priority(); - } + std::for_each(waitables.begin(), waitables.end(), std::mem_fn(&IWaitable::update_priority)); } else if (rc != 0xF601 && rc != 0xE401) { /* TODO: Panic. When can this happen? */ } else { - for (int i = 0; i < handle_index; i++) { - this->waitables[i]->update_priority(); - } + std::for_each(waitables.begin(), waitables.begin() + handle_index, std::mem_fn(&IWaitable::update_priority)); this->waitables.erase(this->waitables.begin() + handle_index); delete w; } @@ -107,4 +102,4 @@ void MultiThreadedWaitableManager::thread_func(void *t) { } } } -} \ No newline at end of file +} diff --git a/source/waitablemanager.cpp b/source/waitablemanager.cpp index fee21cb4..89b89962 100644 --- a/source/waitablemanager.cpp +++ b/source/waitablemanager.cpp @@ -1,6 +1,7 @@ #include #include +#include #include @@ -43,14 +44,10 @@ void WaitableManager::process_internal(bool break_on_timeout) { rc = this->waitables[handle_index]->handle_signaled(0); - for (int i = 0; i < handle_index; i++) { - this->waitables[i]->update_priority(); - } + std::for_each(waitables.begin(), waitables.begin() + handle_index, std::mem_fn(&IWaitable::update_priority)); } else if (rc == 0xEA01) { /* Timeout. */ - for (auto & waitable : this->waitables) { - waitable->update_priority(); - } + std::for_each(waitables.begin(), waitables.end(), std::mem_fn(&IWaitable::update_priority)); if (break_on_timeout) { return; } @@ -72,9 +69,7 @@ void WaitableManager::process_internal(bool break_on_timeout) { /* Delete it. */ delete to_delete; - for (int i = 0; i < handle_index; i++) { - this->waitables[i]->update_priority(); - } + std::for_each(waitables.begin(), waitables.begin() + handle_index, std::mem_fn(&IWaitable::update_priority)); } /* Do deferred callback for each waitable. */ @@ -92,4 +87,4 @@ void WaitableManager::process() { void WaitableManager::process_until_timeout() { WaitableManager::process_internal(true); -} \ No newline at end of file +}