svn commit: r253618 - head/sys/dev/usb/gadget

[Bruce Evans]

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
>>>      <sys/types.h> OR <sys/param.h>, but not both.  <sys/types.h> includes
>>>      <sys/cdefs.h>, and it is okay to depend on that.
>>
>> This is not fully correct.  The consistent style throughout the tree when
>> using _FBSDID() is:
>>
>> #include <sys/cdefs.h>
>> __FBSDID()
>>
>> #include <sys/param.h>
>> ...
>>
>> 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
> <sys/types.h> and <sys/param.h> including <sys/cdefs.h>.
>
> I was one of the ones that put __FBSDID() in much of our code.  I used
> a script to add the two lines:
> 	#include <sys/cdefs.h>
> 	__FBSDID("$FreeBSD$");
>
> I did it this way so as to not break the "but not both" rule for
> <sys/types.h> and <sys/param.h>.  In otherwords my script wasn't
> smart enough to see if <sys/param.h> or <sys/types.h> 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 <sys/param.h>
> 	__FBSDID("$FreeBSD$");
>
> 	#include <sys/___.h>
>
> 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 <sys/types.h>,
> <sys/param.h>, or <sys/cdefs.h> 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 <sys/types.h>	/* 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 <stdio.h>
% @@ -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