Date: Sun, 17 Nov 2002 10:59:51 -0800 From: Luigi Rizzo <luigi@FreeBSD.org> To: Robert Watson <rwatson@FreeBSD.org> 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> In-Reply-To: <Pine.NEB.3.96L.1021117130133.93303D-100000@fledge.watson.org>; from rwatson@FreeBSD.org on Sun, Nov 17, 2002 at 01:15:44PM -0500 References: <200211171630.gAHGUjtH096646@repoman.freebsd.org> <Pine.NEB.3.96L.1021117130133.93303D-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021117105951.A17909>