Kernel: Convert Timer to (mostly) not use Handles

src/core/hle/kernel/timer.cpp

@@ -13,75 +13,54 @@
 
 namespace Kernel {
 
-class Timer : public WaitObject {
-public:
-    std::string GetTypeName() const override { return "Timer"; }
-    std::string GetName() const override { return name; }
+/// The event type of the generic timer callback event
+static int timer_callback_event_type = -1;
 
-    static const HandleType HANDLE_TYPE = HandleType::Timer;
-    HandleType GetHandleType() const override { return HANDLE_TYPE; }
-
-    ResetType reset_type;                   ///< The ResetType of this timer
-
-    bool signaled;                          ///< Whether the timer has been signaled or not
-    std::string name;                       ///< Name of timer (optional)
-
-    u64 initial_delay;                      ///< The delay until the timer fires for the first time
-    u64 interval_delay;                     ///< The delay until the timer fires after the first time
-
-    bool ShouldWait() override {
-        return !signaled;
-    }
-
-    void Acquire() override {
-        _assert_msg_(Kernel, !ShouldWait(), "object unavailable!");
-    }
-};
-
-/**
- * Creates a timer.
- * @param handle Reference to handle for the newly created timer
- * @param reset_type ResetType describing how to create timer
- * @param name Optional name of timer
- * @return Newly created Timer object
- */
-static Timer* CreateTimer(Handle& handle, const ResetType reset_type, const std::string& name) {
-    Timer* timer = new Timer;
-
-    handle = Kernel::g_handle_table.Create(timer).ValueOr(INVALID_HANDLE);
+ResultVal<SharedPtr<Timer>> Timer::Create(ResetType reset_type, std::string name) {
+    SharedPtr<Timer> timer(new Timer);
+    // TOOD(yuriks): Don't create Handle (see Thread::Create())
+    CASCADE_RESULT(auto unused, Kernel::g_handle_table.Create(timer));
 
     timer->reset_type = reset_type;
     timer->signaled = false;
-    timer->name = name;
+    timer->name = std::move(name);
     timer->initial_delay = 0;
     timer->interval_delay = 0;
-    return timer;
+    return MakeResult<SharedPtr<Timer>>(timer);
 }
 
-ResultCode CreateTimer(Handle* handle, const ResetType reset_type, const std::string& name) {
-    CreateTimer(*handle, reset_type, name);
-    return RESULT_SUCCESS;
+bool Timer::ShouldWait() {
+    return !signaled;
 }
 
-ResultCode ClearTimer(Handle handle) {
-    SharedPtr<Timer> timer = Kernel::g_handle_table.Get<Timer>(handle);
-    
-    if (timer == nullptr)
-        return InvalidHandle(ErrorModule::Kernel);
-
-    timer->signaled = false;
-    return RESULT_SUCCESS;
+void Timer::Acquire() {
+    _assert_msg_(Kernel, !ShouldWait(), "object unavailable!");
 }
 
-/// The event type of the generic timer callback event
-static int TimerCallbackEventType = -1;
+void Timer::Set(s64 initial, s64 interval) {
+    initial_delay = initial;
+    interval_delay = interval;
+
+    u64 initial_microseconds = initial / 1000;
+    // TODO(yuriks): Figure out a replacement for GetHandle here
+    CoreTiming::ScheduleEvent(usToCycles(initial_microseconds), timer_callback_event_type,
+            GetHandle());
+}
+
+void Timer::Cancel() {
+    CoreTiming::UnscheduleEvent(timer_callback_event_type, GetHandle());
+}
+
+void Timer::Clear() {
+    signaled = false;
+}
 
 /// The timer callback event, called when a timer is fired
 static void TimerCallback(u64 timer_handle, int cycles_late) {
     SharedPtr<Timer> timer = Kernel::g_handle_table.Get<Timer>(timer_handle);
 
     if (timer == nullptr) {
-        LOG_CRITICAL(Kernel, "Callback fired for invalid timer %u", timer_handle);
+        LOG_CRITICAL(Kernel, "Callback fired for invalid timer %08X", timer_handle);
         return;
     }
 
@@ -99,36 +78,12 @@ static void TimerCallback(u64 timer_handle, int cycles_late) {
         // Reschedule the timer with the interval delay
         u64 interval_microseconds = timer->interval_delay / 1000;
         CoreTiming::ScheduleEvent(usToCycles(interval_microseconds) - cycles_late, 
-                TimerCallbackEventType, timer_handle);
+                timer_callback_event_type, timer_handle);
     }
 }
 
-ResultCode SetTimer(Handle handle, s64 initial, s64 interval) {
-    SharedPtr<Timer> timer = Kernel::g_handle_table.Get<Timer>(handle);
-
-    if (timer == nullptr)
-        return InvalidHandle(ErrorModule::Kernel);
-
-    timer->initial_delay = initial;
-    timer->interval_delay = interval;
-
-    u64 initial_microseconds = initial / 1000;
-    CoreTiming::ScheduleEvent(usToCycles(initial_microseconds), TimerCallbackEventType, handle);
-    return RESULT_SUCCESS;
-}
-
-ResultCode CancelTimer(Handle handle) {
-    SharedPtr<Timer> timer = Kernel::g_handle_table.Get<Timer>(handle);
-
-    if (timer == nullptr)
-        return InvalidHandle(ErrorModule::Kernel);
-
-    CoreTiming::UnscheduleEvent(TimerCallbackEventType, handle);
-    return RESULT_SUCCESS;
-}
-
 void TimersInit() {
-    TimerCallbackEventType = CoreTiming::RegisterEvent("TimerCallback", TimerCallback);
+    timer_callback_event_type = CoreTiming::RegisterEvent("TimerCallback", TimerCallback);
 }
 
 void TimersShutdown() {

src/core/hle/kernel/timer.h

@@ -11,37 +11,50 @@
 
 namespace Kernel {
 
+class Timer : public WaitObject {
+public:
     /**
- * Cancels a timer
- * @param handle Handle of the timer to cancel
+     * Creates a timer
+     * @param reset_type ResetType describing how to create the timer
+     * @param name Optional name of timer
+     * @return The created Timer
      */
-ResultCode CancelTimer(Handle handle);
+    static ResultVal<SharedPtr<Timer>> Create(ResetType reset_type, std::string name = "Unknown");
+
+    std::string GetTypeName() const override { return "Timer"; }
+    std::string GetName() const override { return name; }
+
+    static const HandleType HANDLE_TYPE = HandleType::Timer;
+    HandleType GetHandleType() const override { return HANDLE_TYPE; }
+
+    ResetType reset_type;                   ///< The ResetType of this timer
+
+    bool signaled;                          ///< Whether the timer has been signaled or not
+    std::string name;                       ///< Name of timer (optional)
+
+    u64 initial_delay;                      ///< The delay until the timer fires for the first time
+    u64 interval_delay;                     ///< The delay until the timer fires after the first time
+
+    bool ShouldWait() override;
+    void Acquire() override;
 
     /**
- * Starts a timer with the specified initial delay and interval
- * @param handle Handle of the timer to start
+     * Starts the timer, with the specified initial delay and interval.
      * @param initial Delay until the timer is first fired
      * @param interval Delay until the timer is fired after the first time
      */
-ResultCode SetTimer(Handle handle, s64 initial, s64 interval);
+    void Set(s64 initial, s64 interval);
 
-/**
- * Clears a timer
- * @param handle Handle of the timer to clear
- */
-ResultCode ClearTimer(Handle handle);
+    void Cancel();
+    void Clear();
 
-/**
- * Creates a timer
- * @param handle Handle to the newly created Timer object
- * @param reset_type ResetType describing how to create the timer
- * @param name Optional name of timer
- * @return ResultCode of the error
- */
-ResultCode CreateTimer(Handle* handle, const ResetType reset_type, const std::string& name="Unknown");
+private:
+    Timer() = default;
+};
 
 /// Initializes the required variables for timers
 void TimersInit();
 /// Tears down the timer variables
 void TimersShutdown();
+
 } // namespace

src/core/hle/svc.cpp

@@ -479,28 +479,61 @@ static Result ClearEvent(Handle evt) {
 
 /// Creates a timer
 static Result CreateTimer(Handle* handle, u32 reset_type) {
-    ResultCode res = Kernel::CreateTimer(handle, static_cast<ResetType>(reset_type));
-    LOG_TRACE(Kernel_SVC, "called reset_type=0x%08X : created handle=0x%08X",
-        reset_type, *handle);
-    return res.raw;
+    using Kernel::Timer;
+
+    auto timer_res = Timer::Create(static_cast<ResetType>(reset_type));
+    if (timer_res.Failed())
+        return timer_res.Code().raw;
+
+    auto handle_res = Kernel::g_handle_table.Create(timer_res.MoveFrom());
+    if (handle_res.Failed())
+        return handle_res.Code().raw;
+    *handle = handle_res.MoveFrom();
+
+    LOG_TRACE(Kernel_SVC, "called reset_type=0x%08X : created handle=0x%08X", reset_type, *handle);
+    return RESULT_SUCCESS.raw;
 }
 
 /// Clears a timer
 static Result ClearTimer(Handle handle) {
+    using Kernel::Timer;
+
     LOG_TRACE(Kernel_SVC, "called timer=0x%08X", handle);
-    return Kernel::ClearTimer(handle).raw;
+
+    SharedPtr<Timer> timer = Kernel::g_handle_table.Get<Timer>(handle);
+    if (timer == nullptr)
+        return InvalidHandle(ErrorModule::Kernel).raw;
+
+    timer->Clear();
+    return RESULT_SUCCESS.raw;
 }
 
 /// Starts a timer
 static Result SetTimer(Handle handle, s64 initial, s64 interval) {
+    using Kernel::Timer;
+
     LOG_TRACE(Kernel_SVC, "called timer=0x%08X", handle);
-    return Kernel::SetTimer(handle, initial, interval).raw;
+
+    SharedPtr<Timer> timer = Kernel::g_handle_table.Get<Timer>(handle);
+    if (timer == nullptr)
+        return InvalidHandle(ErrorModule::Kernel).raw;
+
+    timer->Set(initial, interval);
+    return RESULT_SUCCESS.raw;
 }
 
 /// Cancels a timer
 static Result CancelTimer(Handle handle) {
+    using Kernel::Timer;
+
     LOG_TRACE(Kernel_SVC, "called timer=0x%08X", handle);
-    return Kernel::CancelTimer(handle).raw;
+
+    SharedPtr<Timer> timer = Kernel::g_handle_table.Get<Timer>(handle);
+    if (timer == nullptr)
+        return InvalidHandle(ErrorModule::Kernel).raw;
+
+    timer->Cancel();
+    return RESULT_SUCCESS.raw;
 }
 
 /// Sleep the current thread