From cd6ea98890b6975f411ad27223a6045615ea3991 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 7 Apr 2021 01:19:26 -0400 Subject: [PATCH 1/5] k_scheduler: Mark KScopedSchedulerLock as [[nodiscard]] Prevents logic bugs like: KScopedSchedulerLock{kernel}; instead of: KScopedSchedulerLock lk{kernel}; from slipping through. --- src/core/hle/kernel/k_scheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/hle/kernel/k_scheduler.h b/src/core/hle/kernel/k_scheduler.h index f595b9a5c9..01c8c4b733 100644 --- a/src/core/hle/kernel/k_scheduler.h +++ b/src/core/hle/kernel/k_scheduler.h @@ -198,7 +198,7 @@ private: Common::SpinLock guard{}; }; -class KScopedSchedulerLock : KScopedLock { +class [[nodiscard]] KScopedSchedulerLock : KScopedLock { public: explicit KScopedSchedulerLock(KernelCore& kernel); ~KScopedSchedulerLock(); From 923efb53d7b8168493879fefda5397364083ece2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 7 Apr 2021 01:23:06 -0400 Subject: [PATCH 2/5] k_scoped_lock: Mark class as [[nodiscard]] Prevents logic bugs of the kind described in the previous commit from slipping through. --- src/core/hle/kernel/k_scoped_lock.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/hle/kernel/k_scoped_lock.h b/src/core/hle/kernel/k_scoped_lock.h index d7cc557b24..543555680e 100644 --- a/src/core/hle/kernel/k_scoped_lock.h +++ b/src/core/hle/kernel/k_scoped_lock.h @@ -20,7 +20,7 @@ concept KLockable = !std::is_reference_v && requires(T & t) { }; template -requires KLockable class KScopedLock { +requires KLockable class [[nodiscard]] KScopedLock { public: explicit KScopedLock(T* l) : lock_ptr(l) { this->lock_ptr->Lock(); From e295cb04748ede9935b7e1388d0170c551ccdc9e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 7 Apr 2021 01:25:55 -0400 Subject: [PATCH 3/5] k_scoped_lock: delete copy and move assignment operators If we delete the copy and move constructor, we should also be deleting the copy and move assignment operators (and even if this were intended, it would be pretty odd to not document why it's done this way). --- src/core/hle/kernel/k_scoped_lock.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/hle/kernel/k_scoped_lock.h b/src/core/hle/kernel/k_scoped_lock.h index 543555680e..7a1035b8c4 100644 --- a/src/core/hle/kernel/k_scoped_lock.h +++ b/src/core/hle/kernel/k_scoped_lock.h @@ -25,14 +25,17 @@ public: explicit KScopedLock(T* l) : lock_ptr(l) { this->lock_ptr->Lock(); } - explicit KScopedLock(T& l) : KScopedLock(std::addressof(l)) { /* ... */ - } + explicit KScopedLock(T& l) : KScopedLock(std::addressof(l)) {} + ~KScopedLock() { this->lock_ptr->Unlock(); } KScopedLock(const KScopedLock&) = delete; + KScopedLock& operator=(const KScopedLock&) = delete; + KScopedLock(KScopedLock&&) = delete; + KScopedLock& operator=(KScopedLock&&) = delete; private: T* lock_ptr; From 1af23a44b4d4be801d024a9c63a85181fa76ac44 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 7 Apr 2021 01:44:30 -0400 Subject: [PATCH 4/5] k_scoped_scheduler_lock_and_sleep: Mark class as [[nodiscard]] Prevents logic bugs from slipping through. --- src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h b/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h index f8189e1070..4a35842c52 100644 --- a/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h +++ b/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h @@ -15,7 +15,7 @@ namespace Kernel { -class KScopedSchedulerLockAndSleep { +class [[nodiscard]] KScopedSchedulerLockAndSleep { public: explicit KScopedSchedulerLockAndSleep(KernelCore& kernel, KThread* t, s64 timeout) : kernel(kernel), thread(t), timeout_tick(timeout) { From a875393fdbe1406a707248698082ca46d67f2f58 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 7 Apr 2021 01:37:59 -0400 Subject: [PATCH 5/5] Amend bizarre clang-format suggestions --- src/core/hle/kernel/k_scheduler.h | 2 +- src/core/hle/kernel/k_scoped_lock.h | 6 +++--- src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/hle/kernel/k_scheduler.h b/src/core/hle/kernel/k_scheduler.h index 01c8c4b733..70d6bfcee2 100644 --- a/src/core/hle/kernel/k_scheduler.h +++ b/src/core/hle/kernel/k_scheduler.h @@ -200,7 +200,7 @@ private: class [[nodiscard]] KScopedSchedulerLock : KScopedLock { public: - explicit KScopedSchedulerLock(KernelCore& kernel); + explicit KScopedSchedulerLock(KernelCore & kernel); ~KScopedSchedulerLock(); }; diff --git a/src/core/hle/kernel/k_scoped_lock.h b/src/core/hle/kernel/k_scoped_lock.h index 7a1035b8c4..72c3b0252b 100644 --- a/src/core/hle/kernel/k_scoped_lock.h +++ b/src/core/hle/kernel/k_scoped_lock.h @@ -22,10 +22,10 @@ concept KLockable = !std::is_reference_v && requires(T & t) { template requires KLockable class [[nodiscard]] KScopedLock { public: - explicit KScopedLock(T* l) : lock_ptr(l) { + explicit KScopedLock(T * l) : lock_ptr(l) { this->lock_ptr->Lock(); } - explicit KScopedLock(T& l) : KScopedLock(std::addressof(l)) {} + explicit KScopedLock(T & l) : KScopedLock(std::addressof(l)) {} ~KScopedLock() { this->lock_ptr->Unlock(); @@ -34,7 +34,7 @@ public: KScopedLock(const KScopedLock&) = delete; KScopedLock& operator=(const KScopedLock&) = delete; - KScopedLock(KScopedLock&&) = delete; + KScopedLock(KScopedLock &&) = delete; KScopedLock& operator=(KScopedLock&&) = delete; private: diff --git a/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h b/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h index 4a35842c52..ebecf0c778 100644 --- a/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h +++ b/src/core/hle/kernel/k_scoped_scheduler_lock_and_sleep.h @@ -17,7 +17,7 @@ namespace Kernel { class [[nodiscard]] KScopedSchedulerLockAndSleep { public: - explicit KScopedSchedulerLockAndSleep(KernelCore& kernel, KThread* t, s64 timeout) + explicit KScopedSchedulerLockAndSleep(KernelCore & kernel, KThread * t, s64 timeout) : kernel(kernel), thread(t), timeout_tick(timeout) { // Lock the scheduler. kernel.GlobalSchedulerContext().scheduler_lock.Lock();