From owner-svn-src-all@FreeBSD.ORG Mon Oct 10 06:13:15 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D20A0106566C; Mon, 10 Oct 2011 06:13:15 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.64.117]) by mx1.freebsd.org (Postfix) with ESMTP id 412BD8FC0A; Mon, 10 Oct 2011 06:13:15 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.4/8.14.4) with ESMTP id p9A6DDg6054307; Mon, 10 Oct 2011 10:13:13 +0400 (MSD) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.4/8.14.4/Submit) id p9A6DDPX054306; Mon, 10 Oct 2011 10:13:13 +0400 (MSD) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Mon, 10 Oct 2011 10:13:13 +0400 From: Gleb Smirnoff To: Qing Li Message-ID: <20111010061313.GC94905@FreeBSD.org> References: <201110031951.p93JpJLA025249@svn.freebsd.org> <20111009194732.GB94905@glebius.int.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Oct 2011 06:13:15 -0000 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.