Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Oct 2011 10:13:13 +0400
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        Qing Li <qingli@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, bz@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r225947 - head/sys/netinet
Message-ID:  <20111010061313.GC94905@FreeBSD.org>
In-Reply-To: <CAGnGRd%2BUfDYb=3y=8DoeArT7Qp2S1rUyTbQMTjD6cgybyoMKhA@mail.gmail.com>
References:  <201110031951.p93JpJLA025249@svn.freebsd.org> <20111009194732.GB94905@glebius.int.ru> <CAGnGRd%2BUfDYb=3y=8DoeArT7Qp2S1rUyTbQMTjD6cgybyoMKhA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Oct 09, 2011 at 10:11:56PM -0700, Qing Li wrote:
Q> > šWhat confuses me most, is that in lines 1435-1445 you are
Q> > assigning error to a positive value, BUT proceeding further
Q> > with function.
Q> 
Q>    This is what was there before (meaning returning error immediately),
Q>    but I guess a couple of folks felt it looked a bit cluttered.
Q>    This is mostly due to the fact the "RTFREE_LOCKED()" operation
Q>    has to be performed before returning.

Well, we can assign error and then goto "done" label. Assigning error
and continuing processing is confusing, isn't it?

Q> > Well, after third review it is clear, that
Q> > next if() case would definitely be true, and you would proceed
Q> > with return. But that is difficult to see from first glance.
Q> 
Q>    Not so, only for an indirect prefix route.
Q> 
Q> > I'd suggest to remove error variable, return immediately in
Q> > all error cases, and also the RTF_GATEWAY check can be shifted up,
Q> > since it is the most simple and the most usual to be true.
Q> >
Q> 
Q>   No, the RTF_GATEWAY check cannot be shifted up because if we did
Q>   that, the (indirect host route, with destination matching the gateway IP)
Q>   would never be executed, if when that set of conditions are true, which is
Q>   allowed and the reason for the patch in the first place.

Can you elaborate on that please? As far as I see, any rtentry that has
RTF_GATEWAY would return with EINVAL. The first if() clause doesn't
do any actual processing, only checking flags and memcmp()ing. The third
clause either. The error is never reset to 0. So, I don't see any
difference in returning EINVAL for RTF_GATEWAY immediately or later
after other checks.

-- 
Totus tuus, Glebius.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111010061313.GC94905>