Core: Address Feedback

src/core/hardware_properties.h

@@ -8,14 +8,6 @@
 
 namespace Core {
 
-union EmuThreadHandle {
-    u64 raw;
-    struct {
-        u32 host_handle;
-        u32 guest_handle;
-    };
-};
-
 namespace Hardware {
 
 // The below clock rate is based on Switch's clockspeed being widely known as 1.020GHz
@@ -23,6 +15,29 @@ namespace Hardware {
 constexpr u64 BASE_CLOCK_RATE = 1019215872; // Switch cpu frequency is 1020MHz un/docked
 constexpr u64 CNTFREQ = 19200000;           // Switch's hardware clock speed
 constexpr u32 NUM_CPU_CORES = 4;            // Number of CPU Cores
+
 } // namespace Hardware
 
+struct EmuThreadHandle {
+    u32 host_handle;
+    u32 guest_handle;
+
+    u64 GetRaw() const {
+        return (static_cast<u64>(host_handle) << 32) | guest_handle;
+    }
+
+    bool operator==(const EmuThreadHandle& rhs) const {
+        return std::tie(host_handle, guest_handle) == std::tie(rhs.host_handle, rhs.guest_handle);
+    }
+
+    bool operator!=(const EmuThreadHandle& rhs) const {
+        return !operator==(rhs);
+    }
+
+    static constexpr EmuThreadHandle InvalidHandle() {
+        constexpr u32 invalid_handle = 0xFFFFFFFF;
+        return {invalid_handle, invalid_handle};
+    }
+};
+
 } // namespace Core

src/core/hle/kernel/scheduler.cpp

@@ -125,7 +125,7 @@ bool GlobalScheduler::YieldThreadAndBalanceLoad(Thread* yielding_thread) {
     scheduled_queue[core_id].yield(priority);
 
     std::array<Thread*, Core::Hardware::NUM_CPU_CORES> current_threads;
-    for (u32 i = 0; i < Core::Hardware::NUM_CPU_CORES; i++) {
+    for (std::size_t i = 0; i < current_threads.size(); i++) {
         current_threads[i] = scheduled_queue[i].empty() ? nullptr : scheduled_queue[i].front();
     }
 
@@ -178,7 +178,7 @@ bool GlobalScheduler::YieldThreadAndWaitForLoadBalancing(Thread* yielding_thread
     if (scheduled_queue[core_id].empty()) {
         // Here, "current_threads" is calculated after the ""yield"", unlike yield -1
         std::array<Thread*, Core::Hardware::NUM_CPU_CORES> current_threads;
-        for (u32 i = 0; i < Core::Hardware::NUM_CPU_CORES; i++) {
+        for (std::size_t i = 0; i < current_threads.size(); i++) {
             current_threads[i] = scheduled_queue[i].empty() ? nullptr : scheduled_queue[i].front();
         }
         for (auto& thread : suggested_queue[core_id]) {

src/core/hle/kernel/server_session.cpp

@@ -57,7 +57,7 @@ bool ServerSession::IsSignaled() const {
     }
 
     // Wait if we have no pending requests, or if we're currently handling a request.
-    return !(pending_requesting_threads.empty() || currently_handling != nullptr);
+    return !pending_requesting_threads.empty() && currently_handling == nullptr;
 }
 
 void ServerSession::Acquire(Thread* thread) {

src/core/hle/kernel/svc.cpp

@@ -474,7 +474,7 @@ static ResultCode WaitSynchronization(Core::System& system, Handle* index, VAddr
         objects[i] = object;
     }
     auto& synchronization = kernel.Synchronization();
-    auto [result, handle_result] = synchronization.WaitFor(objects, nano_seconds);
+    const auto [result, handle_result] = synchronization.WaitFor(objects, nano_seconds);
     *index = handle_result;
     return result;
 }

src/core/hle/kernel/synchronization.cpp

@@ -4,6 +4,7 @@
 
 #include "core/core.h"
 #include "core/hle/kernel/errors.h"
+#include "core/hle/kernel/handle_table.h"
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/kernel/scheduler.h"
 #include "core/hle/kernel/synchronization.h"
@@ -27,30 +28,30 @@ static bool DefaultThreadWakeupCallback(ThreadWakeupReason reason, std::shared_p
     thread->SetWaitSynchronizationResult(RESULT_SUCCESS);
     thread->SetWaitSynchronizationOutput(static_cast<u32>(index));
     return true;
-};
+}
 
 Synchronization::Synchronization(Core::System& system) : system{system} {}
 
 void Synchronization::SignalObject(SynchronizationObject& obj) const {
     if (obj.IsSignaled()) {
         obj.WakeupAllWaitingThreads();
-    };
+    }
 }
 
 std::pair<ResultCode, Handle> Synchronization::WaitFor(
     std::vector<std::shared_ptr<SynchronizationObject>>& sync_objects, s64 nano_seconds) {
     auto* const thread = system.CurrentScheduler().GetCurrentThread();
     // Find the first object that is acquirable in the provided list of objects
-    auto itr = std::find_if(sync_objects.begin(), sync_objects.end(),
-                            [thread](const std::shared_ptr<SynchronizationObject>& object) {
-                                return object->IsSignaled();
-                            });
+    const auto itr = std::find_if(sync_objects.begin(), sync_objects.end(),
+                                  [thread](const std::shared_ptr<SynchronizationObject>& object) {
+                                      return object->IsSignaled();
+                                  });
 
     if (itr != sync_objects.end()) {
         // We found a ready object, acquire it and set the result value
         SynchronizationObject* object = itr->get();
         object->Acquire(thread);
-        u32 index = static_cast<s32>(std::distance(sync_objects.begin(), itr));
+        const u32 index = static_cast<s32>(std::distance(sync_objects.begin(), itr));
         return {RESULT_SUCCESS, index};
     }
 
@@ -59,12 +60,12 @@ std::pair<ResultCode, Handle> Synchronization::WaitFor(
     // If a timeout value of 0 was provided, just return the Timeout error code instead of
     // suspending the thread.
     if (nano_seconds == 0) {
-        return {RESULT_TIMEOUT, 0};
+        return {RESULT_TIMEOUT, InvalidHandle};
     }
 
     if (thread->IsSyncCancelled()) {
         thread->SetSyncCancelled(false);
-        return {ERR_SYNCHRONIZATION_CANCELED, 0};
+        return {ERR_SYNCHRONIZATION_CANCELED, InvalidHandle};
     }
 
     for (auto& object : sync_objects) {
@@ -80,7 +81,7 @@ std::pair<ResultCode, Handle> Synchronization::WaitFor(
 
     system.PrepareReschedule(thread->GetProcessorID());
 
-    return {RESULT_TIMEOUT, 0};
+    return {RESULT_TIMEOUT, InvalidHandle};
 }
 
 } // namespace Kernel

src/core/hle/kernel/synchronization.h

@@ -6,6 +6,7 @@
 
 #include <memory>
 #include <utility>
+#include <vector>
 
 #include "core/hle/kernel/object.h"
 #include "core/hle/result.h"
@@ -16,15 +17,24 @@ class System;
 
 namespace Kernel {
 
-class KernelCore;
 class SynchronizationObject;
 
+/**
+ * The 'Synchronization' class is an interface for handling synchronization methods
+ * used by Synchronization objects and synchronization SVCs. This centralizes processing of
+ * such
+ */
 class Synchronization {
 public:
-    Synchronization(Core::System& system);
+    explicit Synchronization(Core::System& system);
 
+    /// Signals a synchronization object, waking up all its waiting threads
     void SignalObject(SynchronizationObject& obj) const;
 
+    /// Tries to see if waiting for any of the sync_objects is necessary, if not
+    /// it returns Success and the handle index of the signaled sync object. In
+    /// case not, the current thread will be locked and wait for nano_seconds or
+    /// for a synchronization object to signal.
     std::pair<ResultCode, Handle> WaitFor(
         std::vector<std::shared_ptr<SynchronizationObject>>& sync_objects, s64 nano_seconds);