Merge pull request #8492 from german77/no_more_errors

service: hid: Correct some mistakes and add more validations

src/core/hid/hid_types.h

@@ -272,6 +272,7 @@ enum class VibrationDeviceType : u32 {
     Unknown = 0,
     LinearResonantActuator = 1,
     GcErm = 2,
+    N64 = 3,
 };
 
 // This is nn::hid::VibrationGcErmCommand

src/core/hle/service/hid/controllers/npad.cpp

@@ -49,28 +49,41 @@ bool Controller_NPad::IsNpadIdValid(Core::HID::NpadIdType npad_id) {
     }
 }
 
-bool Controller_NPad::IsDeviceHandleValid(const Core::HID::VibrationDeviceHandle& device_handle) {
+Result Controller_NPad::IsDeviceHandleValid(const Core::HID::VibrationDeviceHandle& device_handle) {
     const auto npad_id = IsNpadIdValid(static_cast<Core::HID::NpadIdType>(device_handle.npad_id));
     const bool npad_type = device_handle.npad_type < Core::HID::NpadStyleIndex::MaxNpadType;
     const bool device_index = device_handle.device_index < Core::HID::DeviceIndex::MaxDeviceIndex;
-    return npad_id && npad_type && device_index;
+
+    if (!npad_type) {
+        return VibrationInvalidStyleIndex;
+    }
+    if (!npad_id) {
+        return VibrationInvalidNpadId;
+    }
+    if (!device_index) {
+        return VibrationDeviceIndexOutOfRange;
+    }
+
+    return ResultSuccess;
 }
 
 Result Controller_NPad::VerifyValidSixAxisSensorHandle(
     const Core::HID::SixAxisSensorHandle& device_handle) {
     const auto npad_id = IsNpadIdValid(static_cast<Core::HID::NpadIdType>(device_handle.npad_id));
+    const bool device_index = device_handle.device_index < Core::HID::DeviceIndex::MaxDeviceIndex;
+    const bool npad_type = device_handle.npad_type < Core::HID::NpadStyleIndex::MaxNpadType;
+
     if (!npad_id) {
         return InvalidNpadId;
     }
-    const bool device_index = device_handle.device_index < Core::HID::DeviceIndex::MaxDeviceIndex;
     if (!device_index) {
         return NpadDeviceIndexOutOfRange;
     }
     // This doesn't get validated on nnsdk
-    const bool npad_type = device_handle.npad_type < Core::HID::NpadStyleIndex::MaxNpadType;
     if (!npad_type) {
         return NpadInvalidHandle;
     }
+
     return ResultSuccess;
 }
 
@@ -705,6 +718,11 @@ Controller_NPad::NpadJoyHoldType Controller_NPad::GetHoldType() const {
 }
 
 void Controller_NPad::SetNpadHandheldActivationMode(NpadHandheldActivationMode activation_mode) {
+    if (activation_mode >= NpadHandheldActivationMode::MaxActivationMode) {
+        ASSERT_MSG(false, "Activation mode should be always None, Single or Dual");
+        return;
+    }
+
     handheld_activation_mode = activation_mode;
 }
 
@@ -840,7 +858,7 @@ bool Controller_NPad::VibrateControllerAtIndex(Core::HID::NpadIdType npad_id,
 void Controller_NPad::VibrateController(
     const Core::HID::VibrationDeviceHandle& vibration_device_handle,
     const Core::HID::VibrationValue& vibration_value) {
-    if (!IsDeviceHandleValid(vibration_device_handle)) {
+    if (IsDeviceHandleValid(vibration_device_handle).IsError()) {
         return;
     }
 
@@ -903,7 +921,7 @@ void Controller_NPad::VibrateControllers(
 
 Core::HID::VibrationValue Controller_NPad::GetLastVibration(
     const Core::HID::VibrationDeviceHandle& vibration_device_handle) const {
-    if (!IsDeviceHandleValid(vibration_device_handle)) {
+    if (IsDeviceHandleValid(vibration_device_handle).IsError()) {
         return {};
     }
 
@@ -914,7 +932,7 @@ Core::HID::VibrationValue Controller_NPad::GetLastVibration(
 
 void Controller_NPad::InitializeVibrationDevice(
     const Core::HID::VibrationDeviceHandle& vibration_device_handle) {
-    if (!IsDeviceHandleValid(vibration_device_handle)) {
+    if (IsDeviceHandleValid(vibration_device_handle).IsError()) {
         return;
     }
 
@@ -941,7 +959,7 @@ void Controller_NPad::SetPermitVibrationSession(bool permit_vibration_session) {
 
 bool Controller_NPad::IsVibrationDeviceMounted(
     const Core::HID::VibrationDeviceHandle& vibration_device_handle) const {
-    if (!IsDeviceHandleValid(vibration_device_handle)) {
+    if (IsDeviceHandleValid(vibration_device_handle).IsError()) {
         return false;
     }
 

src/core/hle/service/hid/controllers/npad.h

@@ -81,6 +81,7 @@ public:
         Dual = 0,
         Single = 1,
         None = 2,
+        MaxActivationMode = 3,
     };
 
     // This is nn::hid::NpadCommunicationMode
@@ -196,7 +197,7 @@ public:
     Core::HID::NpadButton GetAndResetPressState();
 
     static bool IsNpadIdValid(Core::HID::NpadIdType npad_id);
-    static bool IsDeviceHandleValid(const Core::HID::VibrationDeviceHandle& device_handle);
+    static Result IsDeviceHandleValid(const Core::HID::VibrationDeviceHandle& device_handle);
     static Result VerifyValidSixAxisSensorHandle(
         const Core::HID::SixAxisSensorHandle& device_handle);
 

src/core/hle/service/hid/errors.h

@@ -9,6 +9,9 @@ namespace Service::HID {
 
 constexpr Result NpadInvalidHandle{ErrorModule::HID, 100};
 constexpr Result NpadDeviceIndexOutOfRange{ErrorModule::HID, 107};
+constexpr Result VibrationInvalidStyleIndex{ErrorModule::HID, 122};
+constexpr Result VibrationInvalidNpadId{ErrorModule::HID, 123};
+constexpr Result VibrationDeviceIndexOutOfRange{ErrorModule::HID, 124};
 constexpr Result InvalidSixAxisFusionRange{ErrorModule::HID, 423};
 constexpr Result NpadIsDualJoycon{ErrorModule::HID, 601};
 constexpr Result NpadIsSameType{ErrorModule::HID, 602};

src/core/hle/service/hid/hid.cpp

@@ -778,7 +778,7 @@ void Hid::IsSixAxisSensorAtRest(Kernel::HLERequestContext& ctx) {
 
     bool is_at_rest{};
     auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-    const auto result = controller.IsSixAxisSensorAtRest(parameters.sixaxis_handle, is_at_rest);
+    controller.IsSixAxisSensorAtRest(parameters.sixaxis_handle, is_at_rest);
 
     LOG_DEBUG(Service_HID,
               "called, npad_type={}, npad_id={}, device_index={}, applet_resource_user_id={}",
@@ -786,7 +786,7 @@ void Hid::IsSixAxisSensorAtRest(Kernel::HLERequestContext& ctx) {
               parameters.sixaxis_handle.device_index, parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 3};
-    rb.Push(result);
+    rb.Push(ResultSuccess);
     rb.Push(is_at_rest);
 }
 
@@ -803,8 +803,8 @@ void Hid::IsFirmwareUpdateAvailableForSixAxisSensor(Kernel::HLERequestContext& c
 
     bool is_firmware_available{};
     auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-    const auto result = controller.IsFirmwareUpdateAvailableForSixAxisSensor(
+    controller.IsFirmwareUpdateAvailableForSixAxisSensor(parameters.sixaxis_handle,
-        parameters.sixaxis_handle, is_firmware_available);
+                                                         is_firmware_available);
 
     LOG_WARNING(
         Service_HID,
@@ -813,7 +813,7 @@ void Hid::IsFirmwareUpdateAvailableForSixAxisSensor(Kernel::HLERequestContext& c
         parameters.sixaxis_handle.device_index, parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 3};
-    rb.Push(result);
+    rb.Push(ResultSuccess);
     rb.Push(is_firmware_available);
 }
 
@@ -1083,13 +1083,13 @@ void Hid::DisconnectNpad(Kernel::HLERequestContext& ctx) {
     const auto parameters{rp.PopRaw<Parameters>()};
 
     auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-    const auto result = controller.DisconnectNpad(parameters.npad_id);
+    controller.DisconnectNpad(parameters.npad_id);
 
     LOG_DEBUG(Service_HID, "called, npad_id={}, applet_resource_user_id={}", parameters.npad_id,
               parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 2};
-    rb.Push(result);
+    rb.Push(ResultSuccess);
 }
 
 void Hid::GetPlayerLedPattern(Kernel::HLERequestContext& ctx) {
@@ -1165,7 +1165,6 @@ void Hid::SetNpadJoyAssignmentModeSingleByDefault(Kernel::HLERequestContext& ctx
     const auto parameters{rp.PopRaw<Parameters>()};
 
     auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-    const auto result =
     controller.SetNpadMode(parameters.npad_id, Controller_NPad::NpadJoyDeviceType::Left,
                            Controller_NPad::NpadJoyAssignmentMode::Single);
 
@@ -1173,7 +1172,7 @@ void Hid::SetNpadJoyAssignmentModeSingleByDefault(Kernel::HLERequestContext& ctx
              parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 2};
-    rb.Push(result);
+    rb.Push(ResultSuccess);
 }
 
 void Hid::SetNpadJoyAssignmentModeSingle(Kernel::HLERequestContext& ctx) {
@@ -1189,7 +1188,7 @@ void Hid::SetNpadJoyAssignmentModeSingle(Kernel::HLERequestContext& ctx) {
     const auto parameters{rp.PopRaw<Parameters>()};
 
     auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-    const auto result = controller.SetNpadMode(parameters.npad_id, parameters.npad_joy_device_type,
+    controller.SetNpadMode(parameters.npad_id, parameters.npad_joy_device_type,
                            Controller_NPad::NpadJoyAssignmentMode::Single);
 
     LOG_INFO(Service_HID, "called, npad_id={}, applet_resource_user_id={}, npad_joy_device_type={}",
@@ -1197,7 +1196,7 @@ void Hid::SetNpadJoyAssignmentModeSingle(Kernel::HLERequestContext& ctx) {
              parameters.npad_joy_device_type);
 
     IPC::ResponseBuilder rb{ctx, 2};
-    rb.Push(result);
+    rb.Push(ResultSuccess);
 }
 
 void Hid::SetNpadJoyAssignmentModeDual(Kernel::HLERequestContext& ctx) {
@@ -1212,14 +1211,13 @@ void Hid::SetNpadJoyAssignmentModeDual(Kernel::HLERequestContext& ctx) {
     const auto parameters{rp.PopRaw<Parameters>()};
 
     auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-    const auto result = controller.SetNpadMode(parameters.npad_id, {},
+    controller.SetNpadMode(parameters.npad_id, {}, Controller_NPad::NpadJoyAssignmentMode::Dual);
-                                               Controller_NPad::NpadJoyAssignmentMode::Dual);
 
     LOG_INFO(Service_HID, "called, npad_id={}, applet_resource_user_id={}", parameters.npad_id,
              parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 2};
-    rb.Push(result);
+    rb.Push(ResultSuccess);
 }
 
 void Hid::MergeSingleJoyAsDualJoy(Kernel::HLERequestContext& ctx) {
@@ -1412,8 +1410,11 @@ void Hid::ClearNpadCaptureButtonAssignment(Kernel::HLERequestContext& ctx) {
 void Hid::GetVibrationDeviceInfo(Kernel::HLERequestContext& ctx) {
     IPC::RequestParser rp{ctx};
     const auto vibration_device_handle{rp.PopRaw<Core::HID::VibrationDeviceHandle>()};
+    const auto& controller =
+        GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
 
     Core::HID::VibrationDeviceInfo vibration_device_info;
+    bool check_device_index = false;
 
     switch (vibration_device_handle.npad_type) {
     case Core::HID::NpadStyleIndex::ProController:
@@ -1421,17 +1422,22 @@ void Hid::GetVibrationDeviceInfo(Kernel::HLERequestContext& ctx) {
     case Core::HID::NpadStyleIndex::JoyconDual:
     case Core::HID::NpadStyleIndex::JoyconLeft:
     case Core::HID::NpadStyleIndex::JoyconRight:
-    default:
         vibration_device_info.type = Core::HID::VibrationDeviceType::LinearResonantActuator;
+        check_device_index = true;
         break;
     case Core::HID::NpadStyleIndex::GameCube:
         vibration_device_info.type = Core::HID::VibrationDeviceType::GcErm;
         break;
-    case Core::HID::NpadStyleIndex::Pokeball:
+    case Core::HID::NpadStyleIndex::N64:
+        vibration_device_info.type = Core::HID::VibrationDeviceType::N64;
+        break;
+    default:
         vibration_device_info.type = Core::HID::VibrationDeviceType::Unknown;
         break;
     }
 
+    vibration_device_info.position = Core::HID::VibrationDevicePosition::None;
+    if (check_device_index) {
         switch (vibration_device_handle.device_index) {
         case Core::HID::DeviceIndex::Left:
             vibration_device_info.position = Core::HID::VibrationDevicePosition::Left;
@@ -1442,13 +1448,20 @@ void Hid::GetVibrationDeviceInfo(Kernel::HLERequestContext& ctx) {
         case Core::HID::DeviceIndex::None:
         default:
             ASSERT_MSG(false, "DeviceIndex should never be None!");
-        vibration_device_info.position = Core::HID::VibrationDevicePosition::None;
             break;
         }
+    }
 
     LOG_DEBUG(Service_HID, "called, vibration_device_type={}, vibration_device_position={}",
               vibration_device_info.type, vibration_device_info.position);
 
+    const auto result = controller.IsDeviceHandleValid(vibration_device_handle);
+    if (result.IsError()) {
+        IPC::ResponseBuilder rb{ctx, 2};
+        rb.Push(result);
+        return;
+    }
+
     IPC::ResponseBuilder rb{ctx, 4};
     rb.Push(ResultSuccess);
     rb.PushRaw(vibration_device_info);