Bug 81482 – Cross-platform processor core counter: fix build on FreeBSD.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Raphael Kubo da Costa (:rakuco)]

Cross-platform processor core counter: fix build on FreeBSD.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 132530 [details]
Patch

[WebKit Review Bot]

Attachment 132530 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/wtf/NumberOfCores.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #2)
> Attachment 132530 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
> Source/JavaScriptCore/wtf/NumberOfCores.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
> Total errors found: 1 in 2 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

This is a false positive, sys/types.h must come before sys/sysctl.h.

[Zoltan Herczeg]

There is no other OS where this include order caused any trouble. Where is this documentation (a link please)? If this is only FreeBSD specific then add a comment please.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #4)
> There is no other OS where this include order caused any trouble.

It may have broken OpenBSD and NetBSD too, since the requirements are pretty much the same. I don't have access to an OS X machine to check what they do differently for this include order reversal to work (this part of their libraries comes from FreeBSD).

> Where is this documentation (a link please)? If this is only FreeBSD specific then add a comment please.

The man pages for NetBSD, OpenBSD, FreeBSD and Darwin all show that sys/sysctl.h must be included after either sys/param.h or sys/types.h:

 - http://netbsd.gw.com/cgi-bin/man-cgi?sysctl+3+NetBSD-current
 - http://www.openbsd.org/cgi-bin/man.cgi?query=sysctl&sektion=3&arch=i386&apropos=0&manpath=OpenBSD+Current
 - http://www.freebsd.org/cgi/man.cgi?query=sysctl&sektion=3&apropos=0&manpath=FreeBSD+9.0-RELEASE
 - https://developer.apple.com/library/mac/#documentation/Darwin/Reference/Manpages/man3/sysctl.3.html#//apple_ref/doc/man/3/sysctl

u_int is a typedef defined in sys/types.h on FreeBSD (and probably other BSDs too). And including sys/types.h or sys/param.h before is part of the OpenBSD and FreeBSD style guides:

 - http://www.freebsd.org/cgi/man.cgi?query=style&apropos=0&sektion=0&manpath=FreeBSD+9.0-RELEASE&arch=default&format=html
 - http://www.openbsd.org/cgi-bin/man.cgi?query=style&apropos=0&sektion=0&manpath=OpenBSD+Current&arch=i386&format=html

[Zoltan Herczeg]

Poor design :( Headers should include what they need. Do you think this will be fixed in the future?

Ok, if noone objects I can give you an r+ but please upload a new patch with a comment in the header file which clearly states why we break the WK style guides.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 132594 [details]
Same patch with some additional comments in the code

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #6)
> Poor design :( Headers should include what they need. Do you think this will be fixed in the future?

I don't think so, I guess it's part of the design. See select(2) or stat(2) on Linux as well -- they too need additional headers in a certain order.

> Ok, if noone objects I can give you an r+ but please upload a new patch with a comment in the header file which clearly states why we break the WK style guides.

Done.

[WebKit Review Bot]

Attachment 132594 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/wtf/NumberOfCores.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zoltan Herczeg]

Comment on attachment 132594 [details]
Same patch with some additional comments in the code

r=me

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 132594 [details]
Same patch with some additional comments in the code

Clearing flags on attachment: 132594

Committed r111195: <http://trac.webkit.org/changeset/111195>

[Raphael Kubo da Costa (:rakuco)]

All reviewed patches have been landed.  Closing bug.

[Landry Breuil]

Note that on OpenBSD it also needs sys/param.h before sys/sysctl.h and sys/types.h, otherwise some #defines are missing (as stated in comment #5). Should i reopen this bug or file a new one ?

[Zoltan Herczeg]

File a new one.

[Landry Breuil]

#82585.