Date: Thu, 24 Jul 2008 08:54:20 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: Kip Macy <kip.macy@gmail.com> Cc: freebsd-net@freebsd.org, Kip Macy <kmacy@freebsd.org> Subject: Re: moving sockbuf in to its own header Message-ID: <20080724084240.C63347@fledge.watson.org> In-Reply-To: <b1fa29170807201607v52f71cfekd96be0ca0a62fff4@mail.gmail.com> References: <3c1674c90807201514o5bafba72r6be84de6e37a5525@mail.gmail.com> <b1fa29170807201607v52f71cfekd96be0ca0a62fff4@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 20 Jul 2008, Kip Macy wrote: > Actually, I'd like to re-factor multiple parts of socketvar in to separate > files. > > Please provide feedback on the following: > > http://www.fsmware.com/socketvar_refactor.diff This seems like a fairly disruptive change from the perpective of managing future MFCs, and likewise makes it quite a bit harder to diff branches and make sure things haven't been missed. That said, I'm not entirely opposed to it, since I think this decomposition is a fairly reasonable one. Do make sure you've done a complete make universe to hit all the user consumers, such as netstat, etc, that grub around in the kernel parts and make sure there are no surprises. A few comments: - Please propagate the copyright/license from socketvar.h to all derived new files. - You seem to have a lot of extra blank lines -- generally speaking, at most one blank line between pieces of code/comments/etc is required. - The new include files seem not to have forward declarations of the structs referenced from other structures, so in practice you may find that including one requires including the others. Fixing this is easy and, at the very least, non-harmful. It would also lay the way towards not doing nested includes of various includes from socketvar.h in the future. - One of the elements of the BSD style(9) I don't like is the tab between "struct" and "structname" for fields in older structures. Perhaps this is why I notice that it isn't there in the new struct sockbuf line in struct socket, and likewise xsockbuf in xsocket :-) If you do make this change, check in with Peter about whether we now prefer the use of svn copy. Robert N M Watson Computer Laboratory University of Cambridge > > Thanks, > Kip > > > On Sun, Jul 20, 2008 at 3:14 PM, Kip Macy <kmacy@freebsd.org> wrote: >> TOE drivers need to be able to directly enqueue data in to a socket >> buffer and thus benefit from having knowledge of sockbuf internals. >> However, there is no need for them to know about other socket >> definitions. Thus I would like to move sockbuf and accompanying >> definitions to their own header. >> >> This is a fairly straightforward change so I don't really see the need >> to wait more than a few days for feedback: >> >> http://www.fsmware.com/sockbuf.h.diff >> _______________________________________________ >> freebsd-net@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-net >> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >> > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080724084240.C63347>