From 9189c7e706038a508567cb2e46ccdb68b08f4ac7 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 7 Sep 2018 15:26:18 -0700 Subject: checkpatch: Create table of obsolete APIs and apply to RCU This patch creates a deprecated_apis map, which allows such APIs to be flagged with suggested replacements more compactly and straightforwardly. It also uses this map to flag the old flavorful RCU APIs as deprecated, suggesting their vanilla-RCU counterparts as replacements. Signed-off-by: Joe Perches Cc: Andy Whitcroft Signed-off-by: Paul E. McKenney [ paulmck: Merged with earlier less-deft approach. ] --- scripts/checkpatch.pl | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c883ec55654f..dd29e3c28166 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -573,6 +573,27 @@ foreach my $entry (@mode_permission_funcs) { } $mode_perms_search = "(?:${mode_perms_search})"; +our %deprecated_apis = ( + "synchronize_rcu_bh" => "synchronize_rcu", + "synchronize_rcu_bh_expedited" => "synchronize_rcu_expedited", + "call_rcu_bh" => "call_rcu", + "rcu_barrier_bh" => "rcu_barrier", + "synchronize_sched" => "synchronize_rcu", + "synchronize_sched_expedited" => "synchronize_rcu_expedited", + "call_rcu_sched" => "call_rcu", + "rcu_barrier_sched" => "rcu_barrier", + "get_state_synchronize_sched" => "get_state_synchronize_rcu", + "cond_synchronize_sched" => "cond_synchronize_rcu", +); + +#Create a search pattern for all these strings to speed up a loop below +our $deprecated_apis_search = ""; +foreach my $entry (keys %deprecated_apis) { + $deprecated_apis_search .= '|' if ($deprecated_apis_search ne ""); + $deprecated_apis_search .= $entry; +} +$deprecated_apis_search = "(?:${deprecated_apis_search})"; + our $mode_perms_world_writable = qr{ S_IWUGO | S_IWOTH | @@ -6368,6 +6389,14 @@ sub process { "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); } +# check for deprecated apis + if ($line =~ /\b($deprecated_apis_search)\b\s*\(/) { + my $deprecated_api = $1; + my $new_api = $deprecated_apis{$deprecated_api}; + WARN("DEPRECATED_API", + "Deprecated use of '$deprecated_api', prefer '$new_api' instead\n" . $herecurr); + } + # check for various structs that are normally const (ops, kgdb, device_tree) # and avoid what seem like struct definitions 'struct foo {' if ($line !~ /\bconst\b/ && -- cgit v1.2.3 From 3d709ab5a1767d422223e84cf5c20f0d33738274 Mon Sep 17 00:00:00 2001 From: "Paul E. McKenney" Date: Sun, 11 Nov 2018 10:49:10 -0800 Subject: checkpatch.pl: Suggest lockdep instead of asserting !spin_is_locked() This commit points people who might otherwise code up something like WARN_ON(!spin_is_locked(&mylock)) to lockdep_assert_held(&mylock). Signed-off-by: Paul E. McKenney Cc: Andy Whitcroft Cc: Joe Perches --- scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index dd29e3c28166..377f373db6c0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6389,6 +6389,12 @@ sub process { "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); } +# check for spin_is_locked(), suggest lockdep instead + if ($line =~ /\bspin_is_locked\(/) { + WARN("USE_LOCKDEP", + "Where possible, use lockdep_assert_held instead of assertions based on spin_is_locked\n" . $herecurr); + } + # check for deprecated apis if ($line =~ /\b($deprecated_apis_search)\b\s*\(/) { my $deprecated_api = $1; -- cgit v1.2.3 From 77b8c0a8e47484e205b01dfedcd224770aa9d800 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Jan 2019 15:26:59 -0800 Subject: checkpatch: warn on const char foo[] = "bar"; declarations These declarations should generally be static const to avoid poor compilation and runtime performance where compilers tend to initialize the const declaration for every call instead of using .rodata for the string. Miscellanea: - Convert spaces to tabs for indentation in 2 adjacent checks Link: http://lkml.kernel.org/r/10ea5f4b087dc911e41e187a4a2b5e79c7529aa3.camel@perches.com Signed-off-by: Joe Perches Cc: Rasmus Villemoes Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 377f373db6c0..93e84c9504a1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3890,14 +3890,23 @@ sub process { WARN("STATIC_CONST_CHAR_ARRAY", "static const char * array should probably be static const char * const\n" . $herecurr); - } + } + +# check for initialized const char arrays that should be static const + if ($line =~ /^\+\s*const\s+(char|unsigned\s+char|_*u8|(?:[us]_)?int8_t)\s+\w+\s*\[\s*(?:\w+\s*)?\]\s*=\s*"/) { + if (WARN("STATIC_CONST_CHAR_ARRAY", + "const array should probably be static const\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/(^.\s*)const\b/${1}static const/; + } + } # check for static char foo[] = "bar" declarations. if ($line =~ /\bstatic\s+char\s+(\w+)\s*\[\s*\]\s*=\s*"/) { WARN("STATIC_CONST_CHAR_ARRAY", "static char array declaration should probably be static const char\n" . $herecurr); - } + } # check for const const where is not a pointer or array type if ($sline =~ /\bconst\s+($BasicType)\s+const\b/) { -- cgit v1.2.3 From d499480cc435ff153bf7e2e7ee6ab6ac44306c28 Mon Sep 17 00:00:00 2001 From: Jorge Ramirez-Ortiz Date: Thu, 3 Jan 2019 15:29:12 -0800 Subject: checkpatch: add Co-developed-by to signature tags As per Documentation/process/submitting-patches, Co-developed-by is a valid signature. This commit removes the warning. Link: http://lkml.kernel.org/r/1544808928-20002-3-git-send-email-jorge.ramirez-ortiz@linaro.org Signed-off-by: Jorge Ramirez-Ortiz Cc: Greg Kroah-Hartman Cc: Himanshu Jha Cc: Joe Perches Cc: Jonathan Cameron Cc: Jonathan Corbet Cc: Niklas Cassel Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 1 + 1 file changed, 1 insertion(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 93e84c9504a1..b737ca9d7204 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -468,6 +468,7 @@ our $logFunctions = qr{(?x: our $signature_tags = qr{(?xi: Signed-off-by:| + Co-developed-by:| Acked-by:| Tested-by:| Reviewed-by:| -- cgit v1.2.3 From 7967656ffbfa493f5546c0f18bf8a28f702c4baa Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Fri, 18 Jan 2019 15:50:47 -0700 Subject: 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. Link: https://lkml.kernel.org/r/CA+55aFwVZk1OfB9T2v014PTAKFhtVan_Zj2dOjnCy3x6E4UJfA@mail.gmail.com Signed-off-by: Joe Perches Acked-by: Joe Perches Reviewed-by: Bart Van Assche Acked-by: Jani Nikula Reviewed-by: Joey Pabalinas Signed-off-by: Jason Gunthorpe Signed-off-by: Jonathan Corbet --- Documentation/process/coding-style.rst | 38 ++++++++++++++++++++++++++++++---- scripts/checkpatch.pl | 13 ------------ 2 files changed, 34 insertions(+), 17 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst index 4e611a01adc6..8ea913e99fa1 100644 --- a/Documentation/process/coding-style.rst +++ b/Documentation/process/coding-style.rst @@ -938,7 +938,37 @@ result. Typical examples would be functions that return pointers; they use NULL or the ERR_PTR mechanism to report failure. -17) Don't re-invent the kernel macros +17) Using bool +-------------- + +The Linux kernel bool type is an alias for the C99 _Bool type. bool values can +only evaluate to 0 or 1, and implicit or explicit conversion to bool +automatically converts the value to true or false. When using bool types the +!! construction is not needed, which eliminates a class of bugs. + +When working with bool values the true and false definitions should be used +instead of 1 and 0. + +bool function return types and stack variables are always fine to use whenever +appropriate. Use of bool is encouraged to improve readability and is often a +better option than 'int' for storing boolean values. + +Do not use bool if cache line layout or size of the value matters, as its size +and alignment varies based on the compiled architecture. Structures that are +optimized for alignment and size should not use bool. + +If a structure has many true/false values, consider consolidating them into a +bitfield with 1 bit members, or using an appropriate fixed width type, such as +u8. + +Similarly for function arguments, many true/false values can be consolidated +into a single bitwise 'flags' argument and 'flags' can often be a more +readable alternative if the call-sites have naked true/false constants. + +Otherwise limited use of bool in structures and arguments can improve +readability. + +18) Don't re-invent the kernel macros ------------------------------------- The 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 defined that you shouldn't reproduce in your code. -18) Editor modelines and other cruft +19) Editor modelines and other cruft ------------------------------------ Some 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 work correctly. -19) Inline assembly +20) Inline assembly ------------------- In architecture-specific code, you may need to use inline assembly to interface @@ -1027,7 +1057,7 @@ the next instruction in the assembly output: : /* outputs */ : /* inputs */ : /* clobbers */); -20) Conditional Compilation +21) Conditional Compilation --------------------------- Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b737ca9d7204..d62abd032885 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6368,19 +6368,6 @@ sub process { } } -# check for bool bitfields - if ($sline =~ /^.\s+bool\s*$Ident\s*:\s*\d+\s*;/) { - WARN("BOOL_BITFIELD", - "Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>\n" . $herecurr); - } - -# check for bool use in .h files - if ($realfile =~ /\.h$/ && - $sline =~ /^.\s+bool\s*$Ident\s*(?::\s*d+\s*)?;/) { - CHK("BOOL_MEMBER", - "Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384\n" . $herecurr); - } - # check for semaphores initialized locked if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) { WARN("CONSIDER_COMPLETION", -- cgit v1.2.3 From fdf13693d370653013eec3bc7f80a3f535001bf0 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 7 Mar 2019 16:28:32 -0800 Subject: checkpatch: verify SPDX comment style Using SPDX commenting style // or /* is specified for various file types in Documentation/process/license-rules.rst so add an appropriate test for .[chsS] files because many proposed file additions and patches do not use the correct style. Link: http://lkml.kernel.org/r/8b02899853247a2c67669561761f354dd3bd110e.camel@perches.com Signed-off-by: Joe Perches Cc: Rob Herring Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b737ca9d7204..c17cbc06911f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3037,6 +3037,14 @@ sub process { $comment = '..'; } +# check SPDX comment style for .[chsS] files + if ($realfile =~ /\.[chsS]$/ && + $rawline =~ /SPDX-License-Identifier:/ && + $rawline !~ /^\+\s*\Q$comment\E\s*/) { + WARN("SPDX_LICENSE_TAG", + "Improper SPDX comment style for '$realfile', please use '$comment' instead\n" . $herecurr); + } + if ($comment !~ /^$/ && $rawline !~ /^\+\Q$comment\E SPDX-License-Identifier: /) { WARN("SPDX_LICENSE_TAG", -- cgit v1.2.3 From e29a70f1537b1225daceae8d34bf5bfb80546138 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 7 Mar 2019 16:28:35 -0800 Subject: checkpatch: add some new alloc functions to various tests Many new generic allocation functions like the kvmalloc family have been added recently to the kernel. The allocation functions test now includes: o kvmalloc and variants o kstrdup_const o kmemdup_nul o dma_alloc_coherent o alloc_skb and variants Add a separate $allocFunctions variable to help make the allocation functions test a bit more readable. Miscellanea: o Use $allocFunctions in the unnecessary OOM message test and add exclude uses with __GFP_NOWARN o Use $allocFunctions in the unnecessary cast test o Add the kvmalloc family to the preferred sizeof alloc style foo = kvmalloc(sizeof(*foo), ...) Link: http://lkml.kernel.org/r/a5e60a2b93e10baf84af063f6c8e56402273105d.camel@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c17cbc06911f..76748ab23ce9 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -466,6 +466,16 @@ our $logFunctions = qr{(?x: seq_vprintf|seq_printf|seq_puts )}; +our $allocFunctions = qr{(?x: + (?:(?:devm_)? + (?:kv|k|v)[czm]alloc(?:_node|_array)? | + kstrdup(?:_const)? | + kmemdup(?:_nul)?) | + (?:\w+)?alloc_skb(?:ip_align)? | + # dev_alloc_skb/netdev_alloc_skb, et al + dma_alloc_coherent +)}; + our $signature_tags = qr{(?xi: Signed-off-by:| Co-developed-by:| @@ -5553,7 +5563,8 @@ sub process { my ($s, $c) = ctx_statement_block($linenr - 3, $realcnt, 0); # print("line: <$line>\nprevline: <$prevline>\ns: <$s>\nc: <$c>\n\n\n"); - if ($s =~ /(?:^|\n)[ \+]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:\([^\)]*\)\s*)?\s*(?:devm_)?(?:[kv][czm]alloc(?:_node|_array)?\b|kstrdup|kmemdup|(?:dev_)?alloc_skb)/) { + if ($s =~ /(?:^|\n)[ \+]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:\([^\)]*\)\s*)?\s*$allocFunctions\s*\(/ && + $s !~ /\b__GFP_NOWARN\b/ ) { WARN("OOM_MESSAGE", "Possible unnecessary 'out of memory' message\n" . $hereprev); } @@ -6204,8 +6215,8 @@ sub process { } } -# check for pointless casting of kmalloc return - if ($line =~ /\*\s*\)\s*[kv][czm]alloc(_node){0,1}\b/) { +# check for pointless casting of alloc functions + if ($line =~ /\*\s*\)\s*$allocFunctions\b/) { WARN("UNNECESSARY_CASTS", "unnecessary cast may hide bugs, see http://c-faq.com/malloc/mallocnocast.html\n" . $herecurr); } @@ -6213,7 +6224,7 @@ sub process { # alloc style # p = alloc(sizeof(struct foo), ...) should be p = alloc(sizeof(*p), ...) if ($perl_version_ok && - $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*([kv][mz]alloc(?:_node)?)\s*\(\s*(sizeof\s*\(\s*struct\s+$Lval\s*\))/) { + $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc(?:_node)?)\s*\(\s*(sizeof\s*\(\s*struct\s+$Lval\s*\))/) { CHK("ALLOC_SIZEOF_STRUCT", "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } -- cgit v1.2.3 From 98005e8c743f9a5ef3a0e6c971d424da118bd07e Mon Sep 17 00:00:00 2001 From: Vadim Bendebury Date: Thu, 7 Mar 2019 16:28:38 -0800 Subject: checkpatch: allow reporting C99 style comments Presently C99 style comments are removed unconditionally before actual patch validity check happens. This is a problem for some third party projects which use checkpatch.pl but do not allow C99 style comments. This patch adds yet another variable, named C99_COMMENT_TOLERANCE. If it is included in the --ignore command line or config file options list, C99 comments in the patch are reported as errors. Tested by processing a patch with a C99 style comment, it passes the check just fine unless '--ignore C99_COMMENT_TOLERANCE' is present in .checkpatch.conf. Link: http://lkml.kernel.org/r/20190110224957.25008-1-vbendeb@chromium.org Signed-off-by: Vadim Bendebury Cc: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 76748ab23ce9..cbbeb3830a48 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -61,7 +61,7 @@ my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $conststructsfile = "$D/const_structs.checkpatch"; my $typedefsfile = ""; my $color = "auto"; -my $allow_c99_comments = 1; +my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE sub help { my ($exitcode) = @_; @@ -1021,6 +1021,7 @@ if ($git) { } my $vname; +$allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"}; for my $filename (@ARGV) { my $FILE; if ($git) { -- cgit v1.2.3 From a8da38a9cf0e9889a6334bbd6e7364307b571cc1 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 7 Mar 2019 16:28:42 -0800 Subject: checkpatch: add test for SPDX-License-Identifier on wrong line # Warn when any SPDX-License-Identifier: tag is not created on the proper line number. Link: http://lkml.kernel.org/r/9b74ee87f8c1b8fd310e213fcb4994d58610fcb6.camel@perches.com Signed-off-by: Joe Perches Cc: Linus Walleij Cc: "Enrico Weigelt, metux IT consult" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index cbbeb3830a48..8d8d26b5cbbd 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3073,6 +3073,14 @@ sub process { # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); +# check for using SPDX-License-Identifier on the wrong line number + if ($realline != $checklicenseline && + $rawline =~ /\bSPDX-License-Identifier:/ && + substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) { + WARN("SPDX_LICENSE_TAG", + "Misplaced SPDX-License-Identifier tag - use line $checklicenseline instead\n" . $herecurr); + } + # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: -- cgit v1.2.3 From 4462996ea3cc6bcf3c4efbd7bd2514a15dd8ece4 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Thu, 28 Mar 2019 20:44:32 -0700 Subject: checkpatch: add %pt as a valid vsprintf extension Commit 4d42c44727a0 ("lib/vsprintf: Print time and date in human readable format via %pt") introduced a new extension, %pt. Add it in the list of valid extensions. Link: http://lkml.kernel.org/r/20190314203719.29130-1-alexandre.belloni@bootlin.com Signed-off-by: Alexandre Belloni Cc: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 5b756278df13..a09333fd7cef 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5977,7 +5977,7 @@ sub process { while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) { $specifier = $1; $extension = $2; - if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOx]/) { + if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxt]/) { $bad_specifier = $specifier; last; } -- cgit v1.2.3