From owner-svn-src-all@FreeBSD.ORG Tue Jul 30 19:53:58 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id A8963F78; Tue, 30 Jul 2013 19:53:58 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id 3812527DF; Tue, 30 Jul 2013 19:53:57 +0000 (UTC) Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id DA2EBD422E7; Wed, 31 Jul 2013 05:35:37 +1000 (EST) Date: Wed, 31 Jul 2013 05:35:36 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: David O'Brien Subject: Re: svn commit: r253618 - head/sys/dev/usb/gadget In-Reply-To: <20130730190207.GC63635@dragon.NUXI.org> Message-ID: <20130731052029.T2672@besplex.bde.org> References: <201307241832.r6OIWFGc074918@svn.freebsd.org> <201307241529.35175.jhb@freebsd.org> <20130730190207.GC63635@dragon.NUXI.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=DstvpgP+ c=1 sm=1 tr=0 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 a=PO7r1zJSAAAA:8 a=JYkn-E3srG0A:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=4v6nFQ3yUQ4A:10 a=Kt2FaaZfBZeE9Yv_FBQA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, John Baldwin X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Jul 2013 19:53:58 -0000 On Tue, 30 Jul 2013, David O'Brien wrote: > On Wed, Jul 24, 2013 at 03:29:34PM -0400, John Baldwin wrote: >> On Wednesday, July 24, 2013 2:32:15 pm David E. O'Brien wrote: >>> per style(9): >>> Kernel include files (i.e. sys/*.h) come first; normally, include >>> OR , but not both. includes >>> , and it is okay to depend on that. >> >> This is not fully correct. The consistent style throughout the tree when >> using _FBSDID() is: >> >> #include >> __FBSDID() >> >> #include >> ... >> >> Please fix these to match that. It might not be a bad idea to document the >> __FBSDID() practice in style.9 while you are at it. > > Hi John, > As BDE mentioned, the text [still] says it is OK to depend on > and including . > > I was one of the ones that put __FBSDID() in much of our code. I used > a script to add the two lines: > #include > __FBSDID("$FreeBSD$"); > > I did it this way so as to not break the "but not both" rule for > and . In otherwords my script wasn't > smart enough to see if or was already > being included and put the '__FBSDID("$FreeBSD$");' right below it. I tought it was because moving up the include of sys/param.h or sys/types.h to before __FBSDID() would have been more warmly recieved. __FBSDID() is ugly enough even without it distorting the normal include grouping. > I don't feel > #include > __FBSDID("$FreeBSD$"); > > #include > > is against style(9). Of course it is. It would distort the normal include grouping, and there are no examples of this in style(9), and there is an explicit example of using sys/cdefs.h before __FBSDID(). > Should explicit language be added that one of , > , or should be included first > followed by '__FBSDID("$FreeBSD$");' (when used). Followed > by all other headers. Unfortunately, it seems to be necessary to say that every example is intentional. I use the following minor changes which don't fix the text about sys/param.h not actually coming first, but which says more about the intentionality of nearby examples. % Index: style.9 % =================================================================== % RCS file: /home/ncvs/src/share/man/man9/style.9,v % retrieving revision 1.110 % diff -u -2 -r1.110 style.9 % --- style.9 3 Jul 2004 18:29:24 -0000 1.110 % +++ style.9 7 Jul 2004 11:47:22 -0000 % @@ -90,17 +130,22 @@ % .Ed % .Pp % -Leave another blank line before the header files. % +Leave one blank line before the header files. % .Pp % -Kernel include files (i.e.\& % +Kernel include files (i.e.,\& % .Pa sys/*.h ) % -come first; normally, include % -.In sys/types.h % -OR % -.In sys/param.h , % -but not both. % +come first; normally, % .In sys/types.h % +or % +.In sys/param.h % +will be needed before any others. % +.In sys/param.h % includes % +.In sys/types.h . % +Do not include both. % +Many headers, including % +.In sys/types.h , % +include % .In sys/cdefs.h , % -and it is okay to depend on that. % +and it is OK to depend on that. % .Bd -literal % #include /* Non-local includes in angle brackets. */ % @@ -116,12 +161,11 @@ % .Ed % .Pp % -Do not use files in % +Do not include files in % .Pa /usr/include % for files in the kernel. % .Pp % -Leave a blank line before the next group, the % -.Pa /usr/include % -files, % -which should be sorted alphabetically by name. % +Leave a blank line before the next group (XXX nah, all groups), % +the <*.h> include files. % +Sort the <*.h> include files (XXX nah, all groups) alphabetically. % .Bd -literal % #include % @@ -138,5 +182,6 @@ % .Ed % .Pp % -Leave another blank line before the user include files. % +Leave another blank line before the local include files. % +XXX nah, leave it before all groups. % .Bd -literal % #include "pathnames.h" /* Local includes in double quotes. */ Bruce