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 - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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.
This revision is now accepted and ready to land.Dec 4 2015, 7:03 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 :|

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 added a reviewer: bapt.

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 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
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.

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 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

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 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 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.