Thread: Move CreateThread into a static Kernel::Create function

src/core/hle/kernel/thread.cpp

@@ -295,19 +295,41 @@ void DebugThreadQueue() {
     }
 }
 
-/// Creates a new thread
+ResultVal<Handle> Thread::Create(const char* name, u32 entry_point, s32 priority, u32 arg,
-Thread* CreateThread(Handle& handle, const char* name, u32 entry_point, s32 priority,
+        s32 processor_id, u32 stack_top, int stack_size) {
-    s32 processor_id, u32 stack_top, int stack_size) {
+    _dbg_assert_(Kernel, name != nullptr);
 
-    _assert_msg_(KERNEL, (priority >= THREADPRIO_HIGHEST && priority <= THREADPRIO_LOWEST),
+    if ((u32)stack_size < 0x200) {
-        "priority=%d, outside of allowable range!", priority)
+        LOG_ERROR(Kernel, "(name=%s): invalid stack_size=0x%08X", name, stack_size);
+        // TODO: Verify error
+        return ResultCode(ErrorDescription::InvalidSize, ErrorModule::Kernel,
+                ErrorSummary::InvalidArgument, ErrorLevel::Permanent);
+    }
+
+    if (priority < THREADPRIO_HIGHEST || priority > THREADPRIO_LOWEST) {
+        s32 new_priority = CLAMP(priority, THREADPRIO_HIGHEST, THREADPRIO_LOWEST);
+        LOG_WARNING(Kernel_SVC, "(name=%s): invalid priority=%d, clamping to %d",
+            name, priority, new_priority);
+        // TODO(bunnei): Clamping to a valid priority is not necessarily correct behavior... Confirm
+        // validity of this
+        priority = new_priority;
+    }
+
+    if (!Memory::GetPointer(entry_point)) {
+        LOG_ERROR(Kernel_SVC, "(name=%s): invalid entry %08x", name, entry_point);
+        // TODO: Verify error
+        return ResultCode(ErrorDescription::InvalidAddress, ErrorModule::Kernel,
+                ErrorSummary::InvalidArgument, ErrorLevel::Permanent);
+    }
 
     Thread* thread = new Thread;
 
-    // TOOD(yuriks): Fix error reporting
+    ResultVal<Handle> handle = Kernel::g_handle_table.Create(thread);
-    handle = Kernel::g_handle_table.Create(thread).ValueOr(INVALID_HANDLE);
+    // TODO(yuriks): Plug memory leak
+    if (handle.Failed())
+        return handle.Code();
 
-    thread_queue.push_back(handle);
+    thread_queue.push_back(*handle);
     thread_ready_queue.prepare(priority);
 
     thread->thread_id = next_thread_id++;
@@ -322,42 +344,10 @@ Thread* CreateThread(Handle& handle, const char* name, u32 entry_point, s32 prio
     thread->wait_address = 0;
     thread->name = name;
 
-    return thread;
-}
-
-/// Creates a new thread - wrapper for external user
-Handle CreateThread(const char* name, u32 entry_point, s32 priority, u32 arg, s32 processor_id,
-    u32 stack_top, int stack_size) {
-
-    if (name == nullptr) {
-        LOG_ERROR(Kernel_SVC, "nullptr name");
-        return -1;
-    }
-    if ((u32)stack_size < 0x200) {
-        LOG_ERROR(Kernel_SVC, "(name=%s): invalid stack_size=0x%08X", name,
-            stack_size);
-        return -1;
-    }
-    if (priority < THREADPRIO_HIGHEST || priority > THREADPRIO_LOWEST) {
-        s32 new_priority = CLAMP(priority, THREADPRIO_HIGHEST, THREADPRIO_LOWEST);
-        LOG_WARNING(Kernel_SVC, "(name=%s): invalid priority=%d, clamping to %d",
-            name, priority, new_priority);
-        // TODO(bunnei): Clamping to a valid priority is not necessarily correct behavior... Confirm
-        // validity of this
-        priority = new_priority;
-    }
-    if (!Memory::GetPointer(entry_point)) {
-        LOG_ERROR(Kernel_SVC, "(name=%s): invalid entry %08x", name, entry_point);
-        return -1;
-    }
-    Handle handle;
-    Thread* thread = CreateThread(handle, name, entry_point, priority, processor_id, stack_top,
-        stack_size);
-
     ResetThread(thread, arg, 0);
     CallThread(thread);
 
-    return handle;
+    return MakeResult<Handle>(*handle);
 }
 
 /// Get the priority of the thread specified by handle
@@ -409,13 +399,13 @@ ResultCode SetThreadPriority(Handle handle, s32 priority) {
 
 /// Sets up the primary application thread
 Handle SetupMainThread(s32 priority, int stack_size) {
-    Handle handle;
-
     // Initialize new "main" thread
-    Thread* thread = CreateThread(handle, "main", Core::g_app_core->GetPC(), priority,
+    ResultVal<Handle> handle = Thread::Create("main", Core::g_app_core->GetPC(), priority, 0,
         THREADPROCESSORID_0, Memory::SCRATCHPAD_VADDR_END, stack_size);
+    // TODO(yuriks): Propagate error
+    _dbg_assert_(Kernel, handle.Succeeded());
 
-    ResetThread(thread, 0, 0);
+    Thread* thread = Kernel::g_handle_table.Get<Thread>(*handle);
 
     // If running another thread already, set it to "ready" state
     Thread* cur = GetCurrentThread();
@@ -428,7 +418,7 @@ Handle SetupMainThread(s32 priority, int stack_size) {
     thread->status = THREADSTATUS_RUNNING;
     LoadContext(thread->context);
 
-    return handle;
+    return *handle;
 }
 
 

src/core/hle/kernel/thread.h

@@ -53,6 +53,9 @@ namespace Kernel {
 
 class Thread : public Kernel::Object {
 public:
+    static ResultVal<Handle> Create(const char* name, u32 entry_point, s32 priority, u32 arg,
+        s32 processor_id, u32 stack_top, int stack_size = Kernel::DEFAULT_STACK_SIZE);
+
     std::string GetName() const override { return name; }
     std::string GetTypeName() const override { return "Thread"; }
 
@@ -90,17 +93,9 @@ public:
     std::string name;
 
 private:
-    // TODO(yuriks) Temporary until the creation logic can be moved into a static function
-    friend Thread* CreateThread(Handle& handle, const char* name, u32 entry_point, s32 priority,
-            s32 processor_id, u32 stack_top, int stack_size);
-
     Thread() = default;
 };
 
-/// Creates a new thread - wrapper for external user
-Handle CreateThread(const char* name, u32 entry_point, s32 priority, u32 arg, s32 processor_id,
-    u32 stack_top, int stack_size=Kernel::DEFAULT_STACK_SIZE);
-
 /// Sets up the primary application thread
 Handle SetupMainThread(s32 priority, int stack_size=Kernel::DEFAULT_STACK_SIZE);
 

src/core/hle/svc.cpp

@@ -229,14 +229,16 @@ static Result CreateThread(u32 priority, u32 entry_point, u32 arg, u32 stack_top
         name = Common::StringFromFormat("unknown-%08x", entry_point);
     }
 
-    Handle thread = Kernel::CreateThread(name.c_str(), entry_point, priority, arg, processor_id,
+    ResultVal<Handle> thread = Kernel::Thread::Create(name.c_str(), entry_point, priority, arg,
-        stack_top);
+            processor_id, stack_top);
+    if (thread.Failed())
+        return thread.Code().raw;
 
-    Core::g_app_core->SetReg(1, thread);
+    Core::g_app_core->SetReg(1, *thread);
 
     LOG_TRACE(Kernel_SVC, "called entrypoint=0x%08X (%s), arg=0x%08X, stacktop=0x%08X, "
         "threadpriority=0x%08X, processorid=0x%08X : created handle=0x%08X", entry_point,
-        name.c_str(), arg, stack_top, priority, processor_id, thread);
+        name.c_str(), arg, stack_top, priority, processor_id, *thread);
 
     return 0;
 }