Date: Sun, 27 Jul 2008 17:39:07 -0700 From: "Kip Macy" <kip.macy@gmail.com> To: "Robert Watson" <rwatson@freebsd.org> Cc: freebsd-net@freebsd.org Subject: Re: moving sockbuf in to its own header Message-ID: <3c1674c90807271739p3db39ca4r3ed176903f6dae2c@mail.gmail.com> In-Reply-To: <20080724084240.C63347@fledge.watson.org> References: <3c1674c90807201514o5bafba72r6be84de6e37a5525@mail.gmail.com> <b1fa29170807201607v52f71cfekd96be0ca0a62fff4@mail.gmail.com> <20080724084240.C63347@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Robert, I updated the patch to reflect your requested changes. Please confirm that your concerns have been addressed. Thanks, Kip On Thu, Jul 24, 2008 at 12:54 AM, Robert Watson <rwatson@freebsd.org> wrote: > > 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?3c1674c90807271739p3db39ca4r3ed176903f6dae2c>