Re: [PATCH net-next v5] net: ip6mr: add RTM_GETROUTE netlink op

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 9 Jul 2022 14:45:00 +0200 David Lamparter wrote:
> > > +	err = ip6mr_rtm_valid_getroute_req(in_skb, nlh, tb, extack);
> > > +	if (err < 0)
> > > +		goto errout;  
> > 
> > Can we:
> > 
> > 		return err;
> > 
> > ? I don't know where the preference for jumping to the return statement
> > came from, old compilers? someone's "gut feeling"?  
> 
> If I were forced to find a justification, I'd say having a central
> sequence of exit helps avoiding mistakes when some other resource
> acquisition is added later.  Easy to add a cleanup call to an existing
> cleanup block - easy to overlook a "return err;" that needs to be
> changed to "goto errout;".

That only works if the label's name is meaningless, if the label is
named after what it points to you have to rename the label and all the
jumps anyway. Can as well replace returns with a goto.

> But I have absolutely no stake in this at all, I'll happily edit it to
> whatever the consensus is.  This is just what the IPv4 code looks like
> after being adapted for IPv6.

Ah, I looked around other getroute implementations but not specifically
ipmr. I'd rather refactor ipmr.c as well than keep its strangeness.
The fact that we jump to the error path which tries to free the skb
without ever allocating the skb feels particularly off.