Re: [PATCH net] net: ethernet: adi: adin1110: Fix some error handling path in adin1110_read_fifo()

[Simon Horman <horms@kernel.org>]

On Fri, Oct 04, 2024 at 03:15:39PM +0200, Christophe JAILLET wrote:
> Le 04/10/2024 à 13:37, Simon Horman a écrit :
> > On Thu, Oct 03, 2024 at 08:53:15PM +0200, Christophe JAILLET wrote:
> > > If 'frame_size' is too small or if 'round_len' is an error code, it is
> > > likely that an error code should be returned to the caller.
> > > 
> > > Actually, 'ret' is likely to be 0, so if one of these sanity checks fails,
> > > 'success' is returned.
> > 
> > Hi Christophe,
> > 
> > I think we can say "'ret' will be 0".
> 
> Agreed.
> 
> 	ret = adin1110_read_reg()
> 	--> spi_sync_transfer()
> 	--> spi_sync()
> 
> which explicitly documents "zero on success, else a negative error code."
> 
> > At least that is what my brief investigation tells me.
> > 
> > > 
> > > Return -EINVAL instead.
> > 
> 
> If the patch is considered as correct, can you confirm that -EINVAL is the
> correct error code to use? If not, which one would be preferred?

-EINVAL seems reasonable to me.

> > Please include some information on how this was found and tested.
> > e.g.
> > 
> > Found by inspection / Found using widget-ng.
> 
> I would say: found by luck! :)
> 
> The explanation below will be of no help in the commit message and won't be
> added. I just give you all the gory details because you asked for it ;-)
> 
> (and after reading bellow, you can call me crazy!)

:)