From owner-freebsd-net@FreeBSD.ORG Thu Jul 24 07:54:21 2008 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 976A4106566C; Thu, 24 Jul 2008 07:54:21 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42]) by mx1.freebsd.org (Postfix) with ESMTP id 148098FC18; Thu, 24 Jul 2008 07:54:21 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [209.31.154.41]) by cyrus.watson.org (Postfix) with ESMTP id A48B146C7F; Thu, 24 Jul 2008 03:54:20 -0400 (EDT) Date: Thu, 24 Jul 2008 08:54:20 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Kip Macy In-Reply-To: Message-ID: <20080724084240.C63347@fledge.watson.org> References: <3c1674c90807201514o5bafba72r6be84de6e37a5525@mail.gmail.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-net@freebsd.org, Kip Macy Subject: Re: moving sockbuf in to its own header X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Jul 2008 07:54:21 -0000 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 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" >