Re: svn commit: r305998 - in head/usr.bin: cmp indent tr

[Bruce Evans]

On Mon, 19 Sep 2016, John Baldwin wrote:


On Monday, September 19, 2016 01:45:01 PM Ngie Cooper wrote:



On Sep 19, 2016, at 1:43 PM, Conrad E. Meyer  wrote:
Log:
 Move sys/capsicum.h includes after types.h or param.h

 This is not actually documented or even implied in style(9).  Make the change
 to match convention.  Someone should document this convention in style(9).

 Reported by:   jhb
 Sponsored by:  EMC Dell Isilon


Uh??? yes it clearly states it in style(9). From 
https://www.freebsd.org/cgi/man.cgi?query=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.


It doesn't actually say that types.h/param.h has to come before other sys/*.h
headers though.  Normally sys/foo.h requires sys/types.h to compile hence the
rule, but sys/capsicum.h gets around this by a nested include of sys/param.h
(which is itself probably dubious).


This and other nested includes make sys/capsicum.h of rmrf quality.

It is now not so normal to require sys/types or sys/param.h first, since
too many headers have been broken using nested includes.  Only a few have
correct fixes for pollution.  Almost none have documentation for either
their non-pollution or pollution, except possibly with directives like
-D_POSIX_SOURCE (restrict to ~1988 POSIX.1) where the standard has such
documentation and the standard is supported.  E.g., cap_enter(2) only
mentions a couple of symbols and no namespaces.  Its SYNOPSIS uses the
undocumented symbol u_int, and doesn't mention sys/types.h or _BSD_VISIBLE,
so by knowing undocumented things we can tell that its #include of
 is either incomplete or that it has namespace pollution
including at least u_int and probably all the undocumented symbols in
sys/types.h, but that is just the start of the pollution.


I do think we should explicitly add a note to style.9 though to say that
types.h|param.h comes first.


I use the following:

X Index: style.9
X ===
X RCS file: /home/ncvs/src/share/man/man9/style.9,v
X retrieving revision 1.110
X diff -u -2 -r1.110 style.9
X --- style.9   3 Jul 2004 18:29:24 -   1.110
X +++ style.9   7 Jul 2004 11:47:22 -
X @@ -90,17 +130,22 @@
X  .Ed
X  .Pp
X -Leave another blank line before the header files.
X +Leave one blank line before the header files.
X  .Pp
X -Kernel include files (i.e.\&
X +Kernel include files (i.e.,\&
X  .Pa sys/*.h )
X -come first; normally, include
X -.In sys/types.h
X -OR
X -.In sys/param.h ,
X -but not both.
X +come first; normally,
X  .In sys/types.h
X +or
X +.In sys/param.h
X +will be needed before any others.
X +.In sys/param.h
X  includes
X +.In sys/types.h .
X +Do not include both.
X +Many headers, including
X +.In sys/types.h ,
X +include
X  .In sys/cdefs.h ,
X -and it is okay to depend on that.
X +and it is OK to depend on that.
X  .Bd -literal
X  #include  /* Non-local includes in angle brackets. */
X @@ -116,12 +161,11 @@
X  .Ed
X  .Pp
X -Do not use files in
X +Do not include files in
X  .Pa /usr/include
X  for files in the kernel.
X  .Pp
X -Leave a blank line before the next group, the
X -.Pa /usr/include
X -files,
X -which should be sorted alphabetically by name.
X +Leave a blank line before the next group (XXX nah, all groups),
X +the <*.h> include files.
X +Sort the <*.h> include files (XXX nah, all groups) alphabetically.
X  .Bd -literal
X  #include 
X @@ -138,5 +182,6 @@
X  .Ed
X  .Pp
X -Leave another blank line before the user include files.
X +Leave another blank line before the local include files.
X +XXX nah, leave it before all groups.
X  .Bd -literal
X  #include "pathnames.h" /* Local includes in double quotes. */

Bruce___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r305998 - in head/usr.bin: cmp indent tr

[Warner Losh]

If you read style(9) as an annotated example of best practices, you'll
see that it is actually documented there, but poorly. It's the first
one after cdefs for the FreeBSD ID. It says to include one or the
other, with the implication it's first. It's the common interpretation
of the project (this issue has come up before) and it's just one tiny
sliver of tribal knowledge that was attempted to be enshrined there.

Yes, it isn't explicit, but it matches convention in the rest of the
tree. Maybe it should be explicit, but it's always a shit-show when
people tinker with style(9) to make it "clearer" because other people
think you did it wrong, or need to list a big set of exceptions to the
rule or who knows what. Hell, I couldn't even add that {} were allowed
in contexts where people had been using them for a decade without it
being a stupid bikeshed and despite extremely careful vetting and
consensus building I have been told that one committer left the
project over it.

So I'd think twice about modifying style.9 and go have a beer or your
favorite relaxing beverage instead.

Warner

On Mon, Sep 19, 2016 at 3:03 PM, Conrad Meyer  wrote:
> If you re-read the sentences you've pasted carefully, I think you'll
> find it doesn't actually say that the types or param headers come
> before other sys/ headers.  Just that sys/ headers come before
> non-sys/ headers.
>
> Best,
> Conrad
>
> On Mon, Sep 19, 2016 at 1:45 PM, Ngie Cooper (yaneurabeya)
>  wrote:
>>
>> On Sep 19, 2016, at 1:43 PM, Conrad E. Meyer  wrote:
>>
>> Author: cem
>> Date: Mon Sep 19 20:43:03 2016
>> New Revision: 305998
>> URL: https://svnweb.freebsd.org/changeset/base/305998
>>
>> Log:
>>  Move sys/capsicum.h includes after types.h or param.h
>>
>>  This is not actually documented or even implied in style(9).  Make the
>> change
>>  to match convention.  Someone should document this convention in style(9).
>>
>>  Reported by: jhb
>>  Sponsored by: EMC Dell Isilon
>>
>>
>> Uh… yes it clearly states it in style(9). From
>> https://www.freebsd.org/cgi/man.cgi?query=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.
>>
>> Thanks,
>> -Ngie
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r305998 - in head/usr.bin: cmp indent tr

[Warner Losh]

On Mon, Sep 19, 2016 at 2:43 PM, Conrad E. Meyer  wrote:
> Author: cem
> Date: Mon Sep 19 20:43:03 2016
> New Revision: 305998
> URL: https://svnweb.freebsd.org/changeset/base/305998
>
> Log:
>   Move sys/capsicum.h includes after types.h or param.h
>
>   This is not actually documented or even implied in style(9).  Make the 
> change
>   to match convention.  Someone should document this convention in style(9).

style(9) should be read as an annotated example. It comes first in the
example, therefore, it's convention. It is documented, just poorly.
There's many examples in style(9) that are treated this way and it
takes much careful study to understand it. It's tribal knowledge, and
sometimes the only way to learn is to transgress.

Attempts to modify style(9) are fraught with politics, intrigue and
generally a lot of flames even when all that's being done is codifying
existing practice. When I tried to do this last it was a shit storm,
and generally things devolve into a shit storm with no real gain.

Warner
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r305998 - in head/usr.bin: cmp indent tr

[Conrad Meyer]

If you re-read the sentences you've pasted carefully, I think you'll
find it doesn't actually say that the types or param headers come
before other sys/ headers.  Just that sys/ headers come before
non-sys/ headers.

Best,
Conrad

On Mon, Sep 19, 2016 at 1:45 PM, Ngie Cooper (yaneurabeya)
 wrote:
>
> On Sep 19, 2016, at 1:43 PM, Conrad E. Meyer  wrote:
>
> Author: cem
> Date: Mon Sep 19 20:43:03 2016
> New Revision: 305998
> URL: https://svnweb.freebsd.org/changeset/base/305998
>
> Log:
>  Move sys/capsicum.h includes after types.h or param.h
>
>  This is not actually documented or even implied in style(9).  Make the
> change
>  to match convention.  Someone should document this convention in style(9).
>
>  Reported by: jhb
>  Sponsored by: EMC Dell Isilon
>
>
> Uh… yes it clearly states it in style(9). From
> https://www.freebsd.org/cgi/man.cgi?query=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.
>
> Thanks,
> -Ngie
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r305998 - in head/usr.bin: cmp indent tr

[Ngie Cooper (yaneurabeya)]

> On Sep 19, 2016, at 2:22 PM, John Baldwin  wrote:
> 
> On Monday, September 19, 2016 01:45:01 PM Ngie Cooper wrote:
>> 
>>> On Sep 19, 2016, at 1:43 PM, Conrad E. Meyer  wrote:
>>> 
>>> Author: cem
>>> Date: Mon Sep 19 20:43:03 2016
>>> New Revision: 305998
>>> URL: https://svnweb.freebsd.org/changeset/base/305998
>>> 
>>> Log:
>>> Move sys/capsicum.h includes after types.h or param.h
>>> 
>>> This is not actually documented or even implied in style(9).  Make the 
>>> change
>>> to match convention.  Someone should document this convention in style(9).
>>> 
>>> Reported by:jhb
>>> Sponsored by:   EMC Dell Isilon
>> 
>> Uh… yes it clearly states it in style(9). From 
>> https://www.freebsd.org/cgi/man.cgi?query=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.
> 
> It doesn't actually say that types.h/param.h has to come before other sys/*.h
> headers though.  Normally sys/foo.h requires sys/types.h to compile hence the
> rule, but sys/capsicum.h gets around this by a nested include of sys/param.h
> (which is itself probably dubious).
> 
> I do think we should explicitly add a note to style.9 though to say that
> types.h|param.h comes first.

Yeah… I just reread it and I noticed that it’s not 100% explicit.
Thanks,
-Ngie
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r305998 - in head/usr.bin: cmp indent tr

[John Baldwin]

On Monday, September 19, 2016 01:45:01 PM Ngie Cooper wrote:
> 
> > On Sep 19, 2016, at 1:43 PM, Conrad E. Meyer  wrote:
> > 
> > Author: cem
> > Date: Mon Sep 19 20:43:03 2016
> > New Revision: 305998
> > URL: https://svnweb.freebsd.org/changeset/base/305998
> > 
> > Log:
> >  Move sys/capsicum.h includes after types.h or param.h
> > 
> >  This is not actually documented or even implied in style(9).  Make the 
> > change
> >  to match convention.  Someone should document this convention in style(9).
> > 
> >  Reported by:   jhb
> >  Sponsored by:  EMC Dell Isilon
> 
> Uh… yes it clearly states it in style(9). From 
> https://www.freebsd.org/cgi/man.cgi?query=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.

It doesn't actually say that types.h/param.h has to come before other sys/*.h
headers though.  Normally sys/foo.h requires sys/types.h to compile hence the
rule, but sys/capsicum.h gets around this by a nested include of sys/param.h
(which is itself probably dubious).

I do think we should explicitly add a note to style.9 though to say that
types.h|param.h comes first.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r305998 - in head/usr.bin: cmp indent tr

[Ngie Cooper (yaneurabeya)]

> On Sep 19, 2016, at 1:43 PM, Conrad E. Meyer  wrote:
> 
> Author: cem
> Date: Mon Sep 19 20:43:03 2016
> New Revision: 305998
> URL: https://svnweb.freebsd.org/changeset/base/305998
> 
> Log:
>  Move sys/capsicum.h includes after types.h or param.h
> 
>  This is not actually documented or even implied in style(9).  Make the change
>  to match convention.  Someone should document this convention in style(9).
> 
>  Reported by: jhb
>  Sponsored by:EMC Dell Isilon

Uh… yes it clearly states it in style(9). From 
https://www.freebsd.org/cgi/man.cgi?query=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.
Thanks,
-Ngie
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

svn commit: r305998 - in head/usr.bin: cmp indent tr

[Conrad E. Meyer]

Author: cem
Date: Mon Sep 19 20:43:03 2016
New Revision: 305998
URL: https://svnweb.freebsd.org/changeset/base/305998

Log:
  Move sys/capsicum.h includes after types.h or param.h
  
  This is not actually documented or even implied in style(9).  Make the change
  to match convention.  Someone should document this convention in style(9).
  
  Reported by:  jhb
  Sponsored by: EMC Dell Isilon

Modified:
  head/usr.bin/cmp/cmp.c
  head/usr.bin/indent/indent.c
  head/usr.bin/tr/tr.c

Modified: head/usr.bin/cmp/cmp.c
==
--- head/usr.bin/cmp/cmp.c  Mon Sep 19 19:18:40 2016(r305997)
+++ head/usr.bin/cmp/cmp.c  Mon Sep 19 20:43:03 2016(r305998)
@@ -42,8 +42,8 @@ static char sccsid[] = "@(#)cmp.c 8.3 (B
 #include 
 __FBSDID("$FreeBSD$");
 
-#include 
 #include 
+#include 
 #include 
 
 #include 

Modified: head/usr.bin/indent/indent.c
==
--- head/usr.bin/indent/indent.cMon Sep 19 19:18:40 2016
(r305997)
+++ head/usr.bin/indent/indent.cMon Sep 19 20:43:03 2016
(r305998)
@@ -50,8 +50,8 @@ static char sccsid[] = "@(#)indent.c  5.1
 #include 
 __FBSDID("$FreeBSD$");
 
-#include 
 #include 
+#include 
 #include 
 #include 
 #include 

Modified: head/usr.bin/tr/tr.c
==
--- head/usr.bin/tr/tr.cMon Sep 19 19:18:40 2016(r305997)
+++ head/usr.bin/tr/tr.cMon Sep 19 20:43:03 2016(r305998)
@@ -41,8 +41,8 @@ static const char copyright[] =
 static const char sccsid[] = "@(#)tr.c 8.2 (Berkeley) 5/4/95";
 #endif
 
-#include 
 #include 
+#include 
 
 #include 
 #include 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"