History

coding-style: Clarify the expectations around bool

There has been some confusion since checkpatch started warning about bool use in structures, and people have been avoiding using it. Many people feel there is still a legitimate place for bool in structures, so provide some guidance on bool usage derived from the entire thread that spawned the checkpatch warning.

Jason Gunthorpe committed 7 years ago

Commit 79676561 parent b04c11c2 files · +34 −17Browse files

Unified · Split

2 files
  1. Documentation/process/coding-style.rst +34 −4
  2. scripts/checkpatch.pl +0 −13
Documentation/process/coding-style.rst+34−4
@@ -938,7 +938,37 @@ result. Typical examples would be functions that return pointers; they use · 937 unmodified lines
938NULL or the ERR_PTR mechanism to report failure.938NULL or the ERR_PTR mechanism to report failure.
939 939
940 940
94117) Don't re-invent the kernel macros94117) Using bool
942--------------
943
944The Linux kernel bool type is an alias for the C99 _Bool type. bool values can
945only evaluate to 0 or 1, and implicit or explicit conversion to bool
946automatically converts the value to true or false. When using bool types the
947!! construction is not needed, which eliminates a class of bugs.
948
949When working with bool values the true and false definitions should be used
950instead of 1 and 0.
951
952bool function return types and stack variables are always fine to use whenever
953appropriate. Use of bool is encouraged to improve readability and is often a
954better option than 'int' for storing boolean values.
955
956Do not use bool if cache line layout or size of the value matters, as its size
957and alignment varies based on the compiled architecture. Structures that are
958optimized for alignment and size should not use bool.
959
960If a structure has many true/false values, consider consolidating them into a
961bitfield with 1 bit members, or using an appropriate fixed width type, such as
962u8.
963
964Similarly for function arguments, many true/false values can be consolidated
965into a single bitwise 'flags' argument and 'flags' can often be a more
966readable alternative if the call-sites have naked true/false constants.
967
968Otherwise limited use of bool in structures and arguments can improve
969readability.
970
97118) Don't re-invent the kernel macros
942-------------------------------------972-------------------------------------
943 973
944The header file include/linux/kernel.h contains a number of macros that974The header file include/linux/kernel.h contains a number of macros that
@@ -961,7 +991,7 @@ need them. Feel free to peruse that header file to see what else is already · 16 unmodified lines
961defined that you shouldn't reproduce in your code.991defined that you shouldn't reproduce in your code.
962 992
963 993
96418) Editor modelines and other cruft99419) Editor modelines and other cruft
965------------------------------------995------------------------------------
966 996
967Some editors can interpret configuration information embedded in source files,997Some editors can interpret configuration information embedded in source files,
@@ -995,7 +1025,7 @@ own custom mode, or may have some other magic method for making indentation · 27 unmodified lines
995work correctly.1025work correctly.
996 1026
997 1027
99819) Inline assembly102820) Inline assembly
999-------------------1029-------------------
1000 1030
1001In architecture-specific code, you may need to use inline assembly to interface1031In architecture-specific code, you may need to use inline assembly to interface
@@ -1027,7 +1057,7 @@ the next instruction in the assembly output: · 25 unmodified lines
1027 : /* outputs */ : /* inputs */ : /* clobbers */);1057 : /* outputs */ : /* inputs */ : /* clobbers */);
1028 1058
1029 1059
103020) Conditional Compilation106021) Conditional Compilation
1031---------------------------1061---------------------------
1032 1062
1033Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c1063Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c
scripts/checkpatch.pl+0−13
@@ -6368,19 +6368,6 @@ sub process { · 6367 unmodified lines
6368 }6368 }
6369 }6369 }
6370 6370
6371# check for bool bitfields
6372 if ($sline =~ /^.\s+bool\s*$Ident\s*:\s*\d+\s*;/) {
6373 WARN("BOOL_BITFIELD",
6374 "Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>\n" . $herecurr);
6375 }
6376
6377# check for bool use in .h files
6378 if ($realfile =~ /\.h$/ &&
6379 $sline =~ /^.\s+bool\s*$Ident\s*(?::\s*d+\s*)?;/) {
6380 CHK("BOOL_MEMBER",
6381 "Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384\n" . $herecurr);
6382 }
6383
6384# check for semaphores initialized locked6371# check for semaphores initialized locked
6385 if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) {6372 if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) {
6386 WARN("CONSIDER_COMPLETION",6373 WARN("CONSIDER_COMPLETION",