On Wed, Sep 13, 2023 at 04:43:29PM +0200, Bastien Nocera wrote:
> 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.
>
This is a Smatch warning but you have to have built the cross function
database and I only know one other person who does that for sure...
drivers/hid/hid-steelseries.c:393 steelseries_headset_arctis_1_fetch_battery()
warn: error code type promoted to positive: 'ret'
I can add that and resend.
> > 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.
>
I grepped to see which format is more popular and it's 14 vs 13 in favor
of casting. I prefer to avoid casts, but I can resend.
regards,
dan carpenter
