Merge pull request #2425 from Subv/cleanup_todos

Implement some TODOs in the code.

src/core/hle/kernel/resource_limit.cpp

@@ -62,6 +62,8 @@ s32 ResourceLimit::GetCurrentResourceValue(u32 resource) const {
 
 s32 ResourceLimit::GetMaxResourceValue(u32 resource) const {
     switch (resource) {
+    case PRIORITY:
+        return max_priority;
     case COMMIT:
         return max_commit;
     case THREAD:

src/core/hle/kernel/thread.cpp

@@ -353,14 +353,8 @@ static void ResetThreadContext(ARM_Interface::ThreadContext& context, u32 stack_
 
 ResultVal<SharedPtr<Thread>> Thread::Create(std::string name, VAddr entry_point, s32 priority,
                                             u32 arg, s32 processor_id, VAddr stack_top) {
-    if (priority < THREADPRIO_HIGHEST || priority > THREADPRIO_LOWEST) {
-        s32 new_priority = MathUtil::Clamp<s32>(priority, THREADPRIO_HIGHEST, THREADPRIO_LOWEST);
-        LOG_WARNING(Kernel_SVC, "(name=%s): invalid priority=%d, clamping to %d", name.c_str(),
-                    priority, new_priority);
-        // TODO(bunnei): Clamping to a valid priority is not necessarily correct behavior... Confirm
-        // validity of this
-        priority = new_priority;
-    }
+    ASSERT_MSG(priority >= THREADPRIO_HIGHEST && priority <= THREADPRIO_LOWEST,
+               "Invalid thread priority");
 
     if (!Memory::IsValidVirtualAddress(entry_point)) {
         LOG_ERROR(Kernel_SVC, "(name=%s): invalid entry %08x", name.c_str(), entry_point);
@@ -444,25 +438,9 @@ ResultVal<SharedPtr<Thread>> Thread::Create(std::string name, VAddr entry_point,
     return MakeResult<SharedPtr<Thread>>(std::move(thread));
 }
 
-// TODO(peachum): Remove this. Range checking should be done, and an appropriate error should be
-// returned.
-static void ClampPriority(const Thread* thread, s32* priority) {
-    if (*priority < THREADPRIO_HIGHEST || *priority > THREADPRIO_LOWEST) {
-        DEBUG_ASSERT_MSG(
-            false, "Application passed an out of range priority. An error should be returned.");
-
-        s32 new_priority = MathUtil::Clamp<s32>(*priority, THREADPRIO_HIGHEST, THREADPRIO_LOWEST);
-        LOG_WARNING(Kernel_SVC, "(name=%s): invalid priority=%d, clamping to %d",
-                    thread->name.c_str(), *priority, new_priority);
-        // TODO(bunnei): Clamping to a valid priority is not necessarily correct behavior... Confirm
-        // validity of this
-        *priority = new_priority;
-    }
-}
-
 void Thread::SetPriority(s32 priority) {
-    ClampPriority(this, &priority);
-
+    ASSERT_MSG(priority <= THREADPRIO_LOWEST && priority >= THREADPRIO_HIGHEST,
+               "Invalid priority value.");
     // If thread was ready, adjust queues
     if (status == THREADSTATUS_READY)
         ready_queue.move(this, current_priority, priority);

src/core/hle/service/y2r_u.cpp

@@ -531,7 +531,9 @@ static void GetStandardCoefficient(Interface* self) {
         LOG_DEBUG(Service_Y2R, "called standard_coefficient=%u ", index);
     } else {
         cmd_buff[0] = IPC::MakeHeader(0x21, 1, 0);
-        cmd_buff[1] = -1; // TODO(bunnei): Identify the correct error code for this
+        cmd_buff[1] = ResultCode(ErrorDescription::InvalidEnumValue, ErrorModule::CAM,
+                                 ErrorSummary::InvalidArgument, ErrorLevel::Usage)
+                          .raw;
 
         LOG_ERROR(Service_Y2R, "called standard_coefficient=%u  The argument is invalid!", index);
     }

src/core/hle/svc.cpp

@@ -532,16 +532,18 @@ static ResultCode CreateThread(Kernel::Handle* out_handle, s32 priority, u32 ent
         name = Common::StringFromFormat("unknown-%08x", entry_point);
     }
 
-    // TODO(bunnei): Implement resource limits to return an error code instead of the below assert.
-    // The error code should be: Description::NotAuthorized, Module::OS, Summary::WrongArgument,
-    // Level::Permanent
-    ASSERT_MSG(priority >= THREADPRIO_USERLAND_MAX, "Unexpected thread priority!");
-
     if (priority > THREADPRIO_LOWEST) {
         return ResultCode(ErrorDescription::OutOfRange, ErrorModule::OS,
                           ErrorSummary::InvalidArgument, ErrorLevel::Usage);
     }
 
+    using Kernel::ResourceLimit;
+    Kernel::SharedPtr<ResourceLimit>& resource_limit = Kernel::g_current_process->resource_limit;
+    if (resource_limit->GetMaxResourceValue(Kernel::ResourceTypes::PRIORITY) > priority) {
+        return ResultCode(ErrorDescription::NotAuthorized, ErrorModule::OS,
+                          ErrorSummary::WrongArgument, ErrorLevel::Permanent);
+    }
+
     switch (processor_id) {
     case THREADPROCESSORID_ALL:
     case THREADPROCESSORID_DEFAULT:
@@ -598,10 +600,24 @@ static ResultCode GetThreadPriority(s32* priority, Kernel::Handle handle) {
 
 /// Sets the priority for the specified thread
 static ResultCode SetThreadPriority(Kernel::Handle handle, s32 priority) {
+    if (priority > THREADPRIO_LOWEST) {
+        return ResultCode(ErrorDescription::OutOfRange, ErrorModule::OS,
+                          ErrorSummary::InvalidArgument, ErrorLevel::Usage);
+    }
+
     SharedPtr<Kernel::Thread> thread = Kernel::g_handle_table.Get<Kernel::Thread>(handle);
     if (thread == nullptr)
         return ERR_INVALID_HANDLE;
 
+    using Kernel::ResourceLimit;
+    // Note: The kernel uses the current process's resource limit instead of
+    // the one from the thread owner's resource limit.
+    Kernel::SharedPtr<ResourceLimit>& resource_limit = Kernel::g_current_process->resource_limit;
+    if (resource_limit->GetMaxResourceValue(Kernel::ResourceTypes::PRIORITY) > priority) {
+        return ResultCode(ErrorDescription::NotAuthorized, ErrorModule::OS,
+                          ErrorSummary::WrongArgument, ErrorLevel::Permanent);
+    }
+
     thread->SetPriority(priority);
     thread->UpdatePriority();