From 26d03811615e8d021435691c7ae599ac169792ef Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 1 Apr 2019 17:48:31 -0400 Subject: [PATCH 1/4] kernel/thread: Make parameter of GetWaitObjectIndex() const qualified The pointed to member is never actually modified, so it can be made const. --- src/core/hle/kernel/thread.cpp | 4 ++-- src/core/hle/kernel/thread.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index fa3ac3abcb..937b41767f 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -229,9 +229,9 @@ void Thread::SetWaitSynchronizationOutput(s32 output) { context.cpu_registers[1] = output; } -s32 Thread::GetWaitObjectIndex(WaitObject* object) const { +s32 Thread::GetWaitObjectIndex(const WaitObject* object) const { ASSERT_MSG(!wait_objects.empty(), "Thread is not waiting for anything"); - auto match = std::find(wait_objects.rbegin(), wait_objects.rend(), object); + const auto match = std::find(wait_objects.rbegin(), wait_objects.rend(), object); return static_cast(std::distance(match, wait_objects.rend()) - 1); } diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 9c684758c4..1e36a96158 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -205,7 +205,7 @@ public: * object in the list. * @param object Object to query the index of. */ - s32 GetWaitObjectIndex(WaitObject* object) const; + s32 GetWaitObjectIndex(const WaitObject* object) const; /** * Stops a thread, invalidating it from further use From 2d70c30fb2628b4cc3a23b6fda34c2211085b905 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 1 Apr 2019 17:59:43 -0400 Subject: [PATCH 2/4] kernel/thread: Avoid sign conversion within GetCommandBufferAddress() Previously this was performing a u64 + int sign conversion. When dealing with addresses, we should generally be keeping the arithmetic in the same signedness type. This also gets rid of the static lifetime of the constant, as there's no need to make a trivial type like this potentially live for the entire duration of the program. --- src/core/hle/kernel/thread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 937b41767f..332f962877 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -237,8 +237,8 @@ s32 Thread::GetWaitObjectIndex(const WaitObject* object) const { VAddr Thread::GetCommandBufferAddress() const { // Offset from the start of TLS at which the IPC command buffer begins. - static constexpr int CommandHeaderOffset = 0x80; - return GetTLSAddress() + CommandHeaderOffset; + constexpr u64 command_header_offset = 0x80; + return GetTLSAddress() + command_header_offset; } void Thread::SetStatus(ThreadStatus new_status) { From 20cc0b8d3cd46dfc8eed7689f93b6d41f320c709 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 1 Apr 2019 18:19:42 -0400 Subject: [PATCH 3/4] kernel/wait_object: Make ShouldWait() take thread members by pointer-to-const Given this is intended as a querying function, it doesn't make sense to allow the implementer to modify the state of the given thread. --- src/core/hle/kernel/process.cpp | 2 +- src/core/hle/kernel/process.h | 2 +- src/core/hle/kernel/readable_event.cpp | 2 +- src/core/hle/kernel/readable_event.h | 2 +- src/core/hle/kernel/server_port.cpp | 2 +- src/core/hle/kernel/server_port.h | 2 +- src/core/hle/kernel/server_session.cpp | 2 +- src/core/hle/kernel/server_session.h | 2 +- src/core/hle/kernel/thread.cpp | 2 +- src/core/hle/kernel/thread.h | 2 +- src/core/hle/kernel/wait_object.h | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index b0b7af76bd..6e98d78a28 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -247,7 +247,7 @@ void Process::Acquire(Thread* thread) { ASSERT_MSG(!ShouldWait(thread), "Object unavailable!"); } -bool Process::ShouldWait(Thread* thread) const { +bool Process::ShouldWait(const Thread* thread) const { return !is_signaled; } diff --git a/src/core/hle/kernel/process.h b/src/core/hle/kernel/process.h index 732d121706..23f898b0cc 100644 --- a/src/core/hle/kernel/process.h +++ b/src/core/hle/kernel/process.h @@ -237,7 +237,7 @@ private: ~Process() override; /// Checks if the specified thread should wait until this process is available. - bool ShouldWait(Thread* thread) const override; + bool ShouldWait(const Thread* thread) const override; /// Acquires/locks this process for the specified thread if it's available. void Acquire(Thread* thread) override; diff --git a/src/core/hle/kernel/readable_event.cpp b/src/core/hle/kernel/readable_event.cpp index 0e5083f705..c2b798a4e7 100644 --- a/src/core/hle/kernel/readable_event.cpp +++ b/src/core/hle/kernel/readable_event.cpp @@ -14,7 +14,7 @@ namespace Kernel { ReadableEvent::ReadableEvent(KernelCore& kernel) : WaitObject{kernel} {} ReadableEvent::~ReadableEvent() = default; -bool ReadableEvent::ShouldWait(Thread* thread) const { +bool ReadableEvent::ShouldWait(const Thread* thread) const { return !signaled; } diff --git a/src/core/hle/kernel/readable_event.h b/src/core/hle/kernel/readable_event.h index 77a9c362cb..2eb9dcbb78 100644 --- a/src/core/hle/kernel/readable_event.h +++ b/src/core/hle/kernel/readable_event.h @@ -36,7 +36,7 @@ public: return HANDLE_TYPE; } - bool ShouldWait(Thread* thread) const override; + bool ShouldWait(const Thread* thread) const override; void Acquire(Thread* thread) override; /// Unconditionally clears the readable event's state. diff --git a/src/core/hle/kernel/server_port.cpp b/src/core/hle/kernel/server_port.cpp index 0e1515c890..708fdf9e1a 100644 --- a/src/core/hle/kernel/server_port.cpp +++ b/src/core/hle/kernel/server_port.cpp @@ -30,7 +30,7 @@ void ServerPort::AppendPendingSession(SharedPtr pending_session) pending_sessions.push_back(std::move(pending_session)); } -bool ServerPort::ShouldWait(Thread* thread) const { +bool ServerPort::ShouldWait(const Thread* thread) const { // If there are no pending sessions, we wait until a new one is added. return pending_sessions.empty(); } diff --git a/src/core/hle/kernel/server_port.h b/src/core/hle/kernel/server_port.h index 9bc667cf2c..76293cb8b1 100644 --- a/src/core/hle/kernel/server_port.h +++ b/src/core/hle/kernel/server_port.h @@ -75,7 +75,7 @@ public: /// waiting to be accepted by this port. void AppendPendingSession(SharedPtr pending_session); - bool ShouldWait(Thread* thread) const override; + bool ShouldWait(const Thread* thread) const override; void Acquire(Thread* thread) override; private: diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index 4d8a337a76..40cec143e1 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/src/core/hle/kernel/server_session.cpp @@ -46,7 +46,7 @@ ResultVal> ServerSession::Create(KernelCore& kernel, st return MakeResult(std::move(server_session)); } -bool ServerSession::ShouldWait(Thread* thread) const { +bool ServerSession::ShouldWait(const Thread* thread) const { // Closed sessions should never wait, an error will be returned from svcReplyAndReceive. if (parent->client == nullptr) return false; diff --git a/src/core/hle/kernel/server_session.h b/src/core/hle/kernel/server_session.h index aea4ccfebf..79b84baded 100644 --- a/src/core/hle/kernel/server_session.h +++ b/src/core/hle/kernel/server_session.h @@ -82,7 +82,7 @@ public: */ ResultCode HandleSyncRequest(SharedPtr thread); - bool ShouldWait(Thread* thread) const override; + bool ShouldWait(const Thread* thread) const override; void Acquire(Thread* thread) override; diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 332f962877..0e4f6c0411 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -28,7 +28,7 @@ namespace Kernel { -bool Thread::ShouldWait(Thread* thread) const { +bool Thread::ShouldWait(const Thread* thread) const { return status != ThreadStatus::Dead; } diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 1e36a96158..db563708bb 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -111,7 +111,7 @@ public: return HANDLE_TYPE; } - bool ShouldWait(Thread* thread) const override; + bool ShouldWait(const Thread* thread) const override; void Acquire(Thread* thread) override; /** diff --git a/src/core/hle/kernel/wait_object.h b/src/core/hle/kernel/wait_object.h index 5987fb9716..04464a51aa 100644 --- a/src/core/hle/kernel/wait_object.h +++ b/src/core/hle/kernel/wait_object.h @@ -24,7 +24,7 @@ public: * @param thread The thread about which we're deciding. * @return True if the current thread should wait due to this object being unavailable */ - virtual bool ShouldWait(Thread* thread) const = 0; + virtual bool ShouldWait(const Thread* thread) const = 0; /// Acquire/lock the object for the specified thread if it is available virtual void Acquire(Thread* thread) = 0; From 436624173916c2aea8d0ea0e83bebb299da281b2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 1 Apr 2019 18:23:47 -0400 Subject: [PATCH 4/4] kernel/thread: Make AllWaitObjectsReady() a const qualified member function Now that ShouldWait() is a const qualified member function, this one can be made const qualified as well, since it can handle passing a const qualified this pointer to ShouldWait(). --- src/core/hle/kernel/thread.cpp | 2 +- src/core/hle/kernel/thread.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 0e4f6c0411..87638c6551 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -367,7 +367,7 @@ void Thread::ChangeScheduler() { system.CpuCore(processor_id).PrepareReschedule(); } -bool Thread::AllWaitObjectsReady() { +bool Thread::AllWaitObjectsReady() const { return std::none_of( wait_objects.begin(), wait_objects.end(), [this](const SharedPtr& object) { return object->ShouldWait(this); }); diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index db563708bb..73e5d1bb40 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -299,7 +299,7 @@ public: } /// Determines whether all the objects this thread is waiting on are ready. - bool AllWaitObjectsReady(); + bool AllWaitObjectsReady() const; const MutexWaitingThreads& GetMutexWaitingThreads() const { return wait_mutex_threads;