Date: Mon, 30 Apr 2012 15:39:04 +0200 From: Luigi Rizzo <rizzo@iet.unipi.it> To: "Alexander V. Chernikov" <melifaro@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, ae@FreeBSD.org, svn-src-all@FreeBSD.org, Gleb Smirnoff <glebius@FreeBSD.org>, src-committers@FreeBSD.org Subject: Re: svn commit: r234834 - in head/sys: contrib/pf/net net netinet netinet/ipfw ofed/drivers/infiniband/ulp/ipoib Message-ID: <20120430133904.GA65792@onelab2.iet.unipi.it> In-Reply-To: <4F9E82DA.4030406@FreeBSD.org> References: <201204301022.q3UAMNcq060049@svn.freebsd.org> <20120430114836.GA62284@onelab2.iet.unipi.it> <20120430113610.GC18777@FreeBSD.org> <4F9E82DA.4030406@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Apr 30, 2012 at 04:17:30PM +0400, Alexander V. Chernikov wrote: > On 30.04.2012 15:36, Gleb Smirnoff wrote: > >On Mon, Apr 30, 2012 at 01:48:36PM +0200, Luigi Rizzo wrote: > >L> On Mon, Apr 30, 2012 at 10:22:23AM +0000, Alexander V. Chernikov wrote: > >L> > Author: melifaro > >L> > Date: Mon Apr 30 10:22:23 2012 > >L> > New Revision: 234834 > >L> > URL: http://svn.freebsd.org/changeset/base/234834 > >L> > > >L> > Log: > >L> > Move several enums and structures required for L2 filtering from > >ip_fw_private.h to ip_fw.h. > >L> > >L> I would be really grateful if you could revert this back and discuss > >L> what you wanted to achieve with this change other than saving one > >L> entry in the list of includes. > Changing something inside ip_fw_private.h (for example, locking change) > requires changes in several totally unrelated subsystems, which is > clearly bad. There are certainly good reasons to split things even further (as Gleb suggested) so if you want to follow that route and have patches for review i will be glad to discuss that. I am the first one to say that the name ip_fw_private.h is confusing, but at the time i did not feel brave enough to move from a single header (ip_fw.h) to the four (ip_fw.h, ip_fw_user.h, ip_fw_kernel.h, ip_fw_kernel_private.h) that would be needed for proper confinement of information. This said, the change you have made introduce a worse form of header pollution and this is why i am requesting a backout. > >L> > >L> As clearly mentioned in the commit logs > >L> > >L> http://svnweb.freebsd.org/base?view=revision&revision=200580 > Maybe there are some other possibilities documenting preferred layout > other than commit log? Searching 2+yrs commit history is not the best > way of finding information. sure, you could have asked people involved with ipfw development to review this change. Also don't expect a single policy in style(9) to hold for all possible situations: the kernel is huge, it has parts that come from multiple origins in time and space, and sometimes is shared with other OSs as well. Refactoring (such as your changes) should keep that in mind. Let's not make a big deal of this thing: we all make mistakes or have different opinions on how things should be done, and the only way to avoid them is to discuss thing in advance, or be open to resolve things when problems arise. No hard feelings. cheers luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120430133904.GA65792>