linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Hao-Qun Huang <alvinhuang0603@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Viresh Kumar <vireshk@kernel.org>,
	Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
	greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: greybus: hid: fix SET_REPORT return value
Date: Thu, 9 Jul 2026 21:50:40 +0300	[thread overview]
Message-ID: <ak_tgJC8-d1HPKuV@stanley.mountain> (raw)
In-Reply-To: <20260710020644.6728.alvinhuang0603@gmail.com>

On Fri, Jul 10, 2026 at 02:06:44AM +0800, Hao-Qun Huang wrote:
> On Jul 9, 2026 at 6:52 PM, Dan Carpenter <error27@gmail.com> wrote:
> > These kinds of changes require testing.  How have you tested this
> > change?
> 
> I compile-tested it (W=1, building gb-hid, greybus and hid together) and
> traced the return path by hand.  I don't have Greybus HID hardware and
> couldn't find a working emulator (gbsim has been dead since Ara), so I
> haven't run it on a live device.
> 
> The bug is that gb_hid_set_report() returns -errno on failure and len on
> success, and __gb_hid_output_raw_report() computes that into ret (even
> adding one back for the report ID byte) and then returns 0, discarding
> it.  So a successful hidraw write reports 0 bytes written and a failed
> SET_REPORT looks like success.  The GET path next to it already returns
> the count, as do usbhid, i2c-hid and uhid, so callers already handle a
> positive return and greybus HID was the only one returning 0.

The bug is not hard to understand, the issue is that this changes the
function completely...  Was nothing checking the return before?

regards,
dan carpenter


  reply	other threads:[~2026-07-09 18:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-04  8:16 [PATCH] staging: greybus: hid: fix SET_REPORT return value Hao-Qun Huang
2026-07-09 10:52 ` Dan Carpenter
2026-07-09 18:06   ` Hao-Qun Huang
2026-07-09 18:50     ` Dan Carpenter [this message]
2026-07-10  4:40       ` Hao-Qun Huang
2026-07-10  7:42         ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak_tgJC8-d1HPKuV@stanley.mountain \
    --to=error27@gmail.com \
    --cc=alvinhuang0603@gmail.com \
    --cc=elder@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=greybus-dev@lists.linaro.org \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=vireshk@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).