Re: [PATCH] HID: steelseries: Fix signedness bug in steelseries_headset_arctis_1_fetch_battery()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Dan,

On Thu, 2023-09-07 at 12:55 +0300, Dan Carpenter wrote:
> The hid_hw_raw_request() function returns negative error codes or the
> number bytes transferred.  If it returns negative error codes, then
> the
> problem is that when we check if "ret <
> sizeof(arctis_1_battery_request)",
> negative values are type promoted to high unsigned values and treated
> as
> success.  Add an explicit check for negatives to address this issue.

I would be grateful if you could either add the compiler's error
message or explain in greater details the integer type promotion (or
demotion from signed to unsigned in this case) in the commit message.

> Fixes: a0c76896c3fb ("HID: steelseries: Add support for Arctis 1
> XBox")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>



> ---
>  drivers/hid/hid-steelseries.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-
> steelseries.c
> index 43d2cf7153d7..485d2287d58a 100644
> --- a/drivers/hid/hid-steelseries.c
> +++ b/drivers/hid/hid-steelseries.c
> @@ -390,7 +390,7 @@ static int
> steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
>         ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0],
>                                  write_buf,
> sizeof(arctis_1_battery_request),
>                                  HID_OUTPUT_REPORT,
> HID_REQ_SET_REPORT);
> -       if (ret < sizeof(arctis_1_battery_request)) {
> +       if (ret < 0 || ret < sizeof(arctis_1_battery_request)) {

I prefer:
-       if (ret < sizeof(arctis_1_battery_request)) {
+       if (ret < (int) sizeof(arctis_1_battery_request)) {

although I'm not sure that's the kernel-style for this sort of checks.

>                 hid_err(hdev, "hid_hw_raw_request() failed with
> %d\n", ret);
>                 ret = -ENODATA;
>         }




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux