From owner-cvs-all Sun Nov 17 10:59:56 2002 Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id AD97637B415; Sun, 17 Nov 2002 10:59:53 -0800 (PST) Received: from xorpc.icir.org (xorpc.icir.org [192.150.187.68]) by mx1.FreeBSD.org (Postfix) with ESMTP id 49C6143E3B; Sun, 17 Nov 2002 10:59:52 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: from xorpc.icir.org (localhost [127.0.0.1]) by xorpc.icir.org (8.12.3/8.12.3) with ESMTP id gAHIxqAh018693; Sun, 17 Nov 2002 10:59:52 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.3/8.12.3/Submit) id gAHIxpxD018692; Sun, 17 Nov 2002 10:59:51 -0800 (PST) (envelope-from rizzo) Date: Sun, 17 Nov 2002 10:59:51 -0800 From: Luigi Rizzo To: Robert Watson Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet ip_output.c ip_var.h Message-ID: <20021117105951.A17909@xorpc.icir.org> References: <200211171630.gAHGUjtH096646@repoman.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: ; from rwatson@FreeBSD.org on Sun, Nov 17, 2002 at 01:15:44PM -0500 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sun, Nov 17, 2002 at 01:15:44PM -0500, Robert Watson wrote: > I have to say that, on face value, I'm very concerned by this recent > commit spree. The diffs are almost impossible to read since they include I understand your concerns. But the changes are really confined to what the commit log says, no more. Probably diff's output would have looked nicer if i had put the function far away from ip_output() so diff would have been less confused. Sometimes diff is just not the right tool to see what has changed. > I'd actually like to see this backed out -- not because the changes are > necessarily wrong, but because it's difficult to see if they are right. As i said i have no problem in backing it out -- it is very trivial and only affect one file (plus the prototype in ip_var.h), so i thought it was way easier for people to look at a single commit than 5 different ones where several of the steps are just one or a few lines. Also (this in general, not related to this specific commit), what kind of timing do you have in mind between the commits in the sequence below ? 5 minutes, 3 days ? cheers luigi > I'd then like to see them re-introduced carefully in the following > sequence: > > (1) Style cleanup first, including function header changes, line wrapping > changes, register removal, etc. > (2) Separation of IP fragment handling code avoiding funtional and > syntactic changes as much as possible. > (3) Any structural cleanups that can now be performed based on the > factoring out of fragment code. > (4) Variable renaming: earlier, preserve the 'm0' as opposed to 'm' usage > for the fragment code so it's possible to see that nothing changes. > Otherwise it's all one big diff with every line changes. > (5) m_copypacket changes. > > That way if any of these steps introduces bugs, we can easily grab the > source from in between the steps. It will also be easier to understand > the sequence of events. Also, btw, combining all these changes breaks all > outstanding diffs against the code -- if this is done in steps it should > be possible to integrate the changes revision by revision and avoid > serious conflicts. > > I'd like to see this happen for the other changes you've dropped in in the > last few days also: really separate functional changes from style changes, > etc. All seemed good in principle, but almost impossible to read, and > hard to debug if there are any problems. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message