Re: [PATCH v2] i2c: cp2615: handle allocation failure

[Christophe JAILLET]

Le 28/08/2026 à 19:09, Andi Shyti a écrit :
> Hi Triet,
>
> thank you for your patch.
>
> On Tue, Aug 18, 2026 at 11:17:01AM +0000, Triet Hoang wrote:
> > Check the result of kzalloc_obj() and return -ENOMEM
> > when the allocation fails instead of passing a NULL pointer
> > to the message initialization helpers, which would return -EINVAL.
> >
> > Signed-off-by: Triet Hoang <triet.hoang.dev@xxxxxxxxx>
> > ---
> > Changes in v2:
> > - Clarify the commit message to describe the actual behavior change.
> > - Fix coding style regression
>
> Next time please don't send your v2 as in reply to v1. It
> confuses me.
>
> > ---
> >   drivers/i2c/busses/i2c-cp2615.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c
> > index 951de6249834..c1275fcad636 100644
> > --- a/drivers/i2c/busses/i2c-cp2615.c
> > +++ b/drivers/i2c/busses/i2c-cp2615.c
> > @@ -128,6 +128,9 @@ cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w)
> >   	struct usb_device *usbdev = interface_to_usbdev(usbif);
> >   	int res = cp2615_init_i2c_msg(msg, i2c_w);
> >   
> > +	if (!msg)
> > +		return -ENOMEM;
> > +
>
> Your patch looks good, but, as you are at it, can I ask you here

Yes, the patch looks good (and the proposal below even better), but is 
not a must have.

In both cases, the error handling ends being done at [1].

This is not obvious at all when reading the code, but looks safe.

Just my 2c.

CJ


[1]: 
https://elixir.bootlin.com/linux/v7.2/source/drivers/i2c/busses/i2c-cp2615.c#L85


> a little effort? Personally I don't like and I find unreadable
> the form:
>
> 	struct cp2615_iop_msg *msg = kzalloc_obj(*msg);
> 	...
>
> 	if (!msg)
> 		return -ENOMEM.
>
> Important assignments, like kzalloc_*(), shouldn't be made during
> declaration. I prefer the form:
>
> 	
> 	struct cp2615_iop_msg *msg;
> 	...
>
> 	msg = kzalloc_obj(*msg);
> 	if (!msg)
> 		return -ENOMEM.
>
> Works for you? Do you mind updating in v3?
>
> Thanks,
> Andi
>
> >   	if (!res)
> >   		res = usb_bulk_msg(usbdev, usb_sndbulkpipe(usbdev, IOP_EP_OUT),
> >   				   msg, ntohs(msg->length), NULL, 0);