netdev - Re: [PATCH net-next 10/10] ipv6: Refactor __ip6_route

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190410204525.i7qxxpepp6s3y5bv@kafai-mbp.dhcp.thefacebook.com>
Date:   Wed, 10 Apr 2019 20:45:28 +0000
From:   Martin Lau <kafai@...com>
To:     David Ahern <dsahern@...il.com>
CC:     David Ahern <dsahern@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "idosch@...lanox.com" <idosch@...lanox.com>
Subject: Re: [PATCH net-next 10/10] ipv6: Refactor __ip6_route_redirect

On Wed, Apr 10, 2019 at 11:45:04AM -0700, David Ahern wrote:
> >> @@ -2420,7 +2449,7 @@ static struct rt6_info *__ip6_route_redirect(struct net *net,
> >>  					     int flags)
> >>  {
> >>  	struct ip6rd_flowi *rdfl = (struct ip6rd_flowi *)fl6;
> >> -	struct rt6_info *ret = NULL, *rt_cache;
> >> +	struct rt6_info *ret = NULL;
> >>  	struct fib6_info *rt;
> > The "rt" naming is becoming hard to review.
> > It is fortunate that the type context is kept in this diff.
> > 
> > In this local case, the s/rt/f6i/ rename is quite doable?
> 
> I looked into it and I feel such a mass rename just causes noise:
> $ egrep -r 'fib6_info \*rt[,;]' net | wc -l
>       50
> 
> so that's 50 functions that need to be updated and then all rt
> references within the functions. A lot of noise which is why I have not
> sent a name change patch.
Agree that it is a bit noisy (but obvious) change.
I think it was a mistake to decide to postpone this name change while
introducing fib6_info back then.  It would be easier to review if
they were done together:
https://marc.info/?l=linux-netdev&m=151968617902168&w=2

I think a few of us are still ok-ish because we know what
have been going on.  For some, it will be confusing and
it should not be suprised to see this 'struct fib6_info *rt'
being copy-and-pasted to the future patch that makes it
even harder to read/maintain.