Re: [PATCH v2 net-next 10/14] ipv6: Factorise ip6_route_multipath_add().

[Simon Horman <horms@kernel.org>]

On Mon, Apr 14, 2025 at 11:06:58AM -0700, Kuniyuki Iwashima wrote:
> From: Simon Horman <horms@kernel.org>
> Date: Mon, 14 Apr 2025 15:52:26 +0100
> > On Fri, Apr 11, 2025 at 12:33:46PM -0700, Kuniyuki Iwashima wrote:
> > > From: Simon Horman <horms@kernel.org>
> > > Date: Fri, 11 Apr 2025 11:34:04 +0100

...

> > > > Hi Kuniyuki-san,
> > > > 
> > > > Perhaps it can't happen in practice,
> > > 
> > > Yes, it never happens by patch 1 as rtm_to_fib6_multipath_config()
> > > returns an error in such a case.
> > > 
> > > 
> > > > but if the loop above iterates zero
> > > > times then err will be used uninitialised. As it's expected that err is 0
> > > > here, perhaps it would be simplest to just:
> > > > 
> > > > 	return 0;
> > > 
> > > If we want to return 0 above, we need to duplicate list_splice() at
> > > err: and return err; there.  Or initialise err = 0, but this looks
> > > worse to me.
> > 
> > Thanks. I should have dug a bit deeper to determine that this
> > is a false-positive.
> > 
> > > Btw, was this caught by Smatch, Coverity, or something ?  I don't
> > > see such a report at CI.
> > > https://patchwork.kernel.org/project/netdevbpf/patch/20250409011243.26195-11-kuniyu@amazon.com/
> > 
> > Sorry for not mentioning that it was flagged by Smatch,
> > I certainly should have done so.
> 
> Thanks for confirming!
> 
> > 
> > 
> > > 
> > > If so, I'm just curious if we have an official guideline for
> > > false-positives flagged by such tools, like we should care about it
> > > while writing a code and should try to be safer to make it happy.
> > > 
> > > We are also running Coverity for the mainline kernel and have tons
> > > of false-positive reports due to lack of contexts.
> > 
> > I think that the current non-guideline is that we don't change
> > code just to keep the tools happy. Perhaps we should add something
> > about that to the process document?
> 
> Makes sense.
> 
> But looks like the series was marked Changes Requested, not sure
> if it's accidental or intentional, so I'll resend v2 to see others'
> opinion.

I'm not sure either.
But I agree that a v2 is a good way forward.