diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index ff8fddc1d..f0c5e394a 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -295,19 +295,41 @@ void DebugThreadQueue() { } } -/// Creates a new thread -Thread* CreateThread(Handle& handle, const char* name, u32 entry_point, s32 priority, - s32 processor_id, u32 stack_top, int stack_size) { +ResultVal Thread::Create(const char* name, u32 entry_point, s32 priority, u32 arg, + s32 processor_id, u32 stack_top, int stack_size) { + _dbg_assert_(Kernel, name != nullptr); - _assert_msg_(KERNEL, (priority >= THREADPRIO_HIGHEST && priority <= THREADPRIO_LOWEST), - "priority=%d, outside of allowable range!", priority) + if ((u32)stack_size < 0x200) { + 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 - handle = Kernel::g_handle_table.Create(thread).ValueOr(INVALID_HANDLE); + ResultVal handle = Kernel::g_handle_table.Create(thread); + // 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); } /// 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 = 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(*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; } diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 899c14426..542dbdab5 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -53,6 +53,9 @@ namespace Kernel { class Thread : public Kernel::Object { public: + static ResultVal 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); diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index 75a56f10f..72976f655 100644 --- a/src/core/hle/svc.cpp +++ b/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, - stack_top); + ResultVal thread = Kernel::Thread::Create(name.c_str(), entry_point, priority, arg, + 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; }