Messages in this thread | | | Date | Tue, 24 Oct 2023 17:56:47 +0200 (GMT+02:00) | From | Thomas Weißschuh <> | Subject | Re: [PATCH v5 2/2] input: joystick: driver for Adafruit Seesaw Gamepad |
| |
Hi Jeff,
Oct 23, 2023 23:24:55 Jeff LaBundy <jeff@labundy.com>: > On Mon, Oct 23, 2023 at 07:55:52AM +0200, Thomas Weißschuh wrote: > > [...] > >>>> + err = i2c_master_send(client, write_buf, sizeof(write_buf)); >>>> + if (err < 0) >>>> + return err; >>> >>> You correctly return err (or rather, ret) for negative values, but you should also >>> check that ret matches the size of the data sent. For 0 <= ret < sizeof(writebuf), >>> return -EIO. >> >> The driver did this originally. >> I then requested it to be removed as this case >> can never happen. >> i2c_master_send will either return size of(writebuf) or an error. > > Great catch; indeed you are correct. Apologies for having missed this > in the change log; this is good to know in the future.
I guess it would make sense to also adapt the function documentation to be more explicit about this invariant. No need to complicate every caller unnecessarily.
I can send a patch somewhere next week, but if you want to send one I'll be happy to review it.
> That being said, it's a moot point IMO; this driver seems like a good > candidate for regmap. If regmap cannot be made to work here for some > reason, then I'd like to at least see some wrapper functions to avoid > duplicate code and manual assignments to a buffer.
Ack.
Thomas
| |