Page MenuHomeFreeBSD

style.9: Add a small blurb about preferring bool to older types
ClosedPublic

Authored by cem on Dec 4 2015, 6:58 PM.

Details

Summary

No igor regressions.

Suggested by: bde
Sponsored by: EMC / Isilon Storage Division

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem updated this revision to Diff 10747.Dec 4 2015, 6:58 PM
cem retitled this revision from to style.9: Add a small blurb about preferring bool to older types.
cem updated this object.
cem edited the test plan for this revision. (Show Details)
cem added reviewers: wblock, imp.
glebius accepted this revision.Dec 4 2015, 7:03 PM
glebius added a reviewer: glebius.
This revision is now accepted and ready to land.Dec 4 2015, 7:03 PM
cem added a comment.Dec 4 2015, 7:10 PM

+1 from davide on IRC:

11:06 <@davide_> cem: I think the change is good
11:07 <@cem> davide_: Thanks.  Can you sign-off on phabricator?
11:09 <@davide_> cem: I forgot my pwd :|
imp edited edge metadata.Dec 4 2015, 7:16 PM

I think this is good. The case for this is weaker than the u_intXX_t -> uintXX_t, since 'bool' support in compilers is trickier. bool in C++ is way different than one would expect from a 'C' background, so many uses in user-visible interfaces that might be used with C++ should still take a cautious approach and note the considerations.

share/man/man9/style.9
298–299 ↗(On Diff #10747)

old code may be converted if it is reasonable to do so.

bapt accepted this revision.Dec 4 2015, 7:16 PM
bapt added a reviewer: bapt.
imp added a comment.Dec 4 2015, 7:20 PM

more info

share/man/man9/style.9
298–299 ↗(On Diff #10747)

Was going to edit this to say the case for boolean_t and other home-grown types is much, much stronger than for ints. There are many cases where the return value is kinda a bool, but might not always be a bool in the future. Those should be approached with caution.

300–302 ↗(On Diff #10747)

true and false should be preferred to the older spellings of TRUE and FALSE.
stdbool.h should be included (userland). sys/types.h should be used in the kernel.
These files will provide for backward compatibility for any uses of bool when the
compiler is generating code based on older versions of the standard.

cem marked 2 inline comments as done.Dec 4 2015, 7:36 PM
cem added inline comments.
share/man/man9/style.9
300–302 ↗(On Diff #10747)

I'll add a note about TRUE/FALSE.

I don't think it is necessary to note stdbool.h. For example, the fixed-width integers don't suggest using stdint.h in userspace. style(9) takes a light touch; users that don't include the right header will get an easily-Googled compiler warning.

cem updated this revision to Diff 10749.Dec 4 2015, 7:38 PM
cem edited edge metadata.
cem marked an inline comment as done.

Older bool style 'may' be fixed, rather than 'should'; add TRUE/FALSE note.

This revision now requires review to proceed.Dec 4 2015, 7:38 PM
imp added inline comments.Dec 5 2015, 12:23 AM
share/man/man9/style.9
300–302 ↗(On Diff #10749)

The reason I'd add a note here about that is the difference between the kernel and userland.

cem added inline comments.Dec 5 2015, 1:27 AM
share/man/man9/style.9
300–302 ↗(On Diff #10749)

Fair enough. I'll fix it in the next revision. I wonder if now that we have sys/stdint.h, we might as well have sys/stdbool.h too. Not something that will be changed in this patch.

cem updated this revision to Diff 10753.Dec 5 2015, 1:29 AM
cem edited edge metadata.

Add blurb about stdbool.h and sys/types.h, per imp.

cem marked an inline comment as done.Dec 5 2015, 1:30 AM
julian added a subscriber: julian.Dec 5 2015, 1:47 AM

just a nit

share/man/man9/style.9
298 ↗(On Diff #10753)

I Believe that 'former' is used then there are two choices, but you have three.
With three choices "first" would be the wording.

"former + latter" should equal the full set of possibilities (like true or false :-)

cem updated this revision to Diff 10754.Dec 5 2015, 1:54 AM
cem edited edge metadata.

former of 3 -> be specific, per julian.

cem marked an inline comment as done.Dec 5 2015, 1:54 AM
cem added inline comments.
share/man/man9/style.9
298 ↗(On Diff #10754)

Fixed, thanks.

imp accepted this revision.Dec 5 2015, 4:24 PM
imp edited edge metadata.

Thanks for fighting the nitpickers on this (myself included). I think what's there now is good enough to commit. I might pick a nit here or there, but I doubt I'd make it better doing so at this point.

This revision is now accepted and ready to land.Dec 5 2015, 4:24 PM
This revision was automatically updated to reflect the committed changes.
cem marked an inline comment as done.