Re: [PATCH] staging: greybus: hid: fix SET_REPORT return value

[Dan Carpenter]

So the patch has already been merged and it's fine.  The fix is
correct.

On Fri, Jul 10, 2026 at 12:40:50PM +0800, Hao-Qun Huang wrote:
> On Jul 10, 2026 at 2:50 AM, Dan Carpenter <error27@xxxxxxxxx> wrote:
> > The bug is not hard to understand, the issue is that this changes the
> > function completely...  Was nothing checking the return before?
> 
> It gets checked.  The value flows unchanged through __hid_hw_raw_request()
> to the callers, and they look at it two ways:
> 
>  - hidraw returns it straight to userspace (write(), HIDIOCSFEATURE),
>    where it is the number of bytes transferred.
> 
>  - in-kernel SET_REPORT callers, some testing "ret < 0" (hid-multitouch,
>    hid-sony), some testing "ret != size" (hid-gt683r, hid-lenovo,
>    hid-razer).
> 
> So the old return 0 was wrong both ways: the first group had a failed
> SET_REPORT masked as success, and the second saw every SET_REPORT as a
> failure.  Returning the count or a negative errno is what GET already
> does in this driver and what usbhid/i2c-hid/uhid return, so nothing
> working with those relied on the 0.

What I'm trying to say is, sure, it's easy to see the code is buggy but
it's been that way for years.  Your patch changes the return completely
from always returning zero to never returning zero.  When we're reviewing
this patch we want to know how making that change is safe.

In staging often the answer is that nothing was calling that function and
we can delete it...

But here, the real answer is that almost nothing checks for errors.  For
the few places that do, almost all of them only check for negatives.
That's probably how the code was able to work as it is...  The commit
message it should explain the risks.

"Changing this code is fine because almost nothing checks for errors.
There are a one or two in kernel checks which care about the exact
positive return and this patch will fix that but basically not much is
affected.  And hopefully userspace doesn't check either or it only checks
for negative errors.  But in the spirit of correctness, lets change this
return to be return the number of bytes and if userspace needs adjusting
we will deal with that when users file their bug reports."

regards,
dan carpenter