diff options
Diffstat (limited to 'scripts/checkpatch.pl')
| -rwxr-xr-x | scripts/checkpatch.pl | 220 |
1 files changed, 178 insertions, 42 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7b28ad331742..0492d6afc9a1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -113,7 +113,8 @@ Options: --max-line-length=n set the maximum line length, (default $max_line_length) if exceeded, warn on patches requires --strict for use with --file - --min-conf-desc-length=n set the min description length, if shorter, warn + --min-conf-desc-length=n set the minimum description length for config symbols + in lines, if shorter, warn (default $min_conf_desc_length) --tab-size=n set the number of spaces for tab (default $tabsize) --root=PATH PATH to the kernel tree root --no-summary suppress the per-file summary @@ -150,6 +151,24 @@ EOM exit($exitcode); } +my $DO_WHILE_0_ADVICE = q{ + do {} while (0) advice is over-stated in a few situations: + + The more obvious case is macros, like MODULE_PARM_DESC, invoked at + file-scope, where C disallows code (it must be in functions). See + $exceptions if you have one to add by name. + + More troublesome is declarative macros used at top of new scope, + like DECLARE_PER_CPU. These might just compile with a do-while-0 + wrapper, but would be incorrect. Most of these are handled by + detecting struct,union,etc declaration primitives in $exceptions. + + Theres also macros called inside an if (block), which "return" an + expression. These cannot do-while, and need a ({}) wrapper. + + Enjoy this qualification while we work to improve our heuristics. +}; + sub uniq { my %seen; return grep { !$seen{$_}++ } @_; @@ -622,6 +641,7 @@ our $signature_tags = qr{(?xi: Reviewed-by:| Reported-by:| Suggested-by:| + Assisted-by:| To:| Cc: )}; @@ -666,6 +686,9 @@ our $tracing_logging_tags = qr{(?xi: [\.\!:\s]* )}; +# Device ID types like found in include/linux/mod_devicetable.h. +our $dev_id_types = qr{\b[a-z]\w*_device_id\b}; + sub edit_distance_min { my (@arr) = @_; my $len = scalar @arr; @@ -838,6 +861,12 @@ our %deprecated_apis = ( "kunmap" => "kunmap_local", "kmap_atomic" => "kmap_local_page", "kunmap_atomic" => "kunmap_local", + #These should be enough to drive away new IDR users + "DEFINE_IDR" => "DEFINE_XARRAY", + "idr_init" => "xa_init", + "idr_init_base" => "xa_init_flags", + "rcu_read_lock_trace" => "rcu_read_lock_tasks_trace", + "rcu_read_unlock_trace" => "rcu_read_unlock_tasks_trace", ); #Create a search pattern for all these strings to speed up a loop below @@ -1074,7 +1103,9 @@ our $declaration_macros = qr{(?x: (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(| (?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(| (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\(| - (?:$Storage\s+)?(?:XA_STATE|XA_STATE_ORDER)\s*\( + (?:$Storage\s+)?(?:XA_STATE|XA_STATE_ORDER)\s*\(| + __cacheline_group_(?:begin|end)(?:_aligned)?\s*\(| + __dma_from_device_group_(?:begin|end)\s*\( )}; our %allow_repeated_words = ( @@ -2614,6 +2645,11 @@ sub exclude_global_initialisers { $realfile =~ m@/bpf/.*\.bpf\.c$@; } +sub is_userspace { + my ($realfile) = @_; + return ($realfile =~ m@^tools/@ || $realfile =~ m@^scripts/@); +} + sub process { my $filename = shift; @@ -2893,7 +2929,7 @@ sub process { } $checklicenseline = 1; - if ($realfile !~ /^MAINTAINERS/) { + if ($realfile !~ /^(MAINTAINERS|dev\/null)/) { my $last_binding_patch = $is_binding_patch; $is_binding_patch = () = $realfile =~ m@^(?:Documentation/devicetree/|include/dt-bindings/)@; @@ -3000,6 +3036,16 @@ sub process { } } +# Check for invalid patch separator + if ($in_commit_log && + $line =~ /^---.+/) { + if (ERROR("BAD_COMMIT_SEPARATOR", + "Invalid commit separator - some tools may have problems applying this\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/-/=/g; + } + } + # Check for patch separator if ($line =~ /^---$/) { $has_patch_separator = 1; @@ -3060,6 +3106,15 @@ sub process { } } + # Assisted-by uses AGENT_NAME:MODEL_VERSION format, not email + if ($sign_off =~ /^Assisted-by:/i) { + if ($email !~ /^\S+:\S+/) { + WARN("BAD_SIGN_OFF", + "Assisted-by expects 'AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]' format\n" . $herecurr); + } + next; + } + my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); my $suggested_email = format_email(($email_name, $name_comment, $email_address, $comment)); if ($suggested_email eq "") { @@ -3272,7 +3327,7 @@ sub process { # file delta changes $line =~ /^\s*(?:[\w\.\-\+]*\/)++[\w\.\-\+]+:/ || # filename then : - $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i || + $line =~ /^\s*(?:Fixes:|https?:|$link_tags_search|$signature_tags)/i || # A Fixes:, link or signature tag line $commit_log_possible_stack_dump)) { WARN("COMMIT_LOG_LONG_LINE", @@ -3318,6 +3373,13 @@ sub process { } } +# Check for auto-generated unhandled placeholder text (mostly for cover letters) + if (($in_commit_log || $in_header_lines) && + $rawline =~ /(?:SUBJECT|BLURB) HERE/) { + ERROR("PLACEHOLDER_USE", + "Placeholder text detected\n" . $herecurr); + } + # Check for git id commit length and improperly formed commit descriptions # A correctly formed commit description is: # commit <SHA-1 hash length 12+ chars> ("Complete commit subject") @@ -3481,9 +3543,10 @@ sub process { # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { - while ($rawline =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { + my $rawline_utf8 = decode("utf8", $rawline); + while ($rawline_utf8 =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { my $typo = $1; - my $blank = copy_spacing($rawline); + my $blank = copy_spacing($rawline_utf8); my $ptr = substr($blank, 0, $-[1]) . "^" x length($typo); my $hereptr = "$hereline$ptr\n"; my $typo_fix = $spelling_fix{lc($typo)}; @@ -3645,7 +3708,7 @@ sub process { $help_length < $min_conf_desc_length) { my $stat_real = get_stat_real($linenr, $ln - 1); WARN("CONFIG_DESCRIPTION", - "please write a help paragraph that fully describes the config symbol\n" . "$here\n$stat_real\n"); + "please write a help paragraph that fully describes the config symbol with at least $min_conf_desc_length lines\n" . "$here\n$stat_real\n"); } } @@ -3689,20 +3752,6 @@ sub process { } } - if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && - ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { - my $flag = $1; - my $replacement = { - 'EXTRA_AFLAGS' => 'asflags-y', - 'EXTRA_CFLAGS' => 'ccflags-y', - 'EXTRA_CPPFLAGS' => 'cppflags-y', - 'EXTRA_LDFLAGS' => 'ldflags-y', - }; - - WARN("DEPRECATED_VARIABLE", - "Use of $flag is deprecated, please use \`$replacement->{$flag} instead.\n" . $herecurr) if ($replacement->{$flag}); - } - # check for DT compatible documentation if (defined $root && (($realfile =~ /\.dtsi?$/ && $line =~ /^\+\s*compatible\s*=\s*\"/) || @@ -3734,6 +3783,18 @@ sub process { } } +# Check for RGMII phy-mode with delay on PCB + if ($realfile =~ /\.(dts|dtsi|dtso)$/ && + $line =~ /^\+\s*(phy-mode|phy-connection-type)\s*=\s*"/ && + !ctx_has_comment($first_line, $linenr)) { + my $prop = $1; + my $mode = get_quoted_string($line, $rawline); + if ($mode =~ /^"rgmii(?:|-rxid|-txid)"$/) { + WARN("UNCOMMENTED_RGMII_MODE", + "$prop $mode without comment -- delays on the PCB should be described, otherwise use \"rgmii-id\"\n" . $herecurr); + } + } + # check for using SPDX license tag at beginning of files if ($realline == $checklicenseline) { if ($rawline =~ /^[ \+]\s*\#\!\s*\//) { @@ -3805,6 +3866,14 @@ sub process { "Misplaced SPDX-License-Identifier tag - use line $checklicenseline instead\n" . $herecurr); } +# check for disallowed SPDX file tags + if ($rawline =~ /\bSPDX-.*:/ && + $rawline !~ /\bSPDX-License-Identifier:/ && + $rawline !~ /\bSPDX-FileCopyrightText:/) { + WARN("SPDX_LICENSE_TAG", + "Disallowed SPDX tag\n" . $herecurr); + } + # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: @@ -4817,7 +4886,7 @@ sub process { } # do not use BUG() or variants - if ($line =~ /\b(?!AA_|BUILD_|DCCP_|IDA_|KVM_|RWLOCK_|snd_|SPIN_)(?:[a-zA-Z_]*_)?BUG(?:_ON)?(?:_[A-Z_]+)?\s*\(/) { + if ($line =~ /\b(?!AA_|BUILD_|IDA_|KVM_|RWLOCK_|snd_|SPIN_)(?:[a-zA-Z_]*_)?BUG(?:_ON)?(?:_[A-Z_]+)?\s*\(/) { my $msg_level = \&WARN; $msg_level = \&CHK if ($file); &{$msg_level}("AVOID_BUG", @@ -5896,9 +5965,9 @@ sub process { } } -# multi-statement macros should be enclosed in a do while loop, grab the -# first statement and ensure its the whole macro if its not enclosed -# in a known good container +# Usually multi-statement macros should be enclosed in a do {} while +# (0) loop. Grab the first statement and ensure its the whole macro +# if its not enclosed in a known good container if ($realfile !~ m@/vmlinux.lds.h$@ && $line =~ /^.\s*\#\s*define\s*$Ident(\()?/) { my $ln = $linenr; @@ -5951,10 +6020,13 @@ sub process { my $exceptions = qr{ $Declare| + # named exceptions module_param_named| MODULE_PARM_DESC| DECLARE_PER_CPU| DEFINE_PER_CPU| + static_assert| + # declaration primitives __typeof__\(| union| struct| @@ -5989,11 +6061,11 @@ sub process { ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", "Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects\n" . "$herectx"); } elsif ($dstat =~ /;/) { - ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", - "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx"); + WARN("MULTISTATEMENT_MACRO_USE_DO_WHILE", + "Non-declarative macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx\nBUT SEE:\n$DO_WHILE_0_ADVICE"); } else { ERROR("COMPLEX_MACRO", - "Macros with complex values should be enclosed in parentheses\n" . "$herectx"); + "Macros with complex values should be enclosed in parentheses\n" . "$herectx\nBUT SEE:\n$DO_WHILE_0_ADVICE"); } } @@ -6037,7 +6109,7 @@ sub process { } # check if this is an unused argument - if ($define_stmt !~ /\b$arg\b/) { + if ($define_stmt !~ /\b$arg\b/ && $define_stmt) { WARN("MACRO_ARG_UNUSED", "Argument '$arg' is not used in function-like macro\n" . "$herectx"); } @@ -6693,6 +6765,13 @@ sub process { } } +# check for context_unsafe without a comment. + if ($line =~ /\bcontext_unsafe\b/ && + !ctx_has_comment($first_line, $linenr)) { + WARN("CONTEXT_UNSAFE", + "context_unsafe without comment\n" . $herecurr); + } + # check of hardware specific defines if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { CHK("ARCH_DEFINES", @@ -6904,7 +6983,7 @@ sub process { ($extension eq "f" && defined $qualifier && $qualifier !~ /^w/) || ($extension eq "4" && - defined $qualifier && $qualifier !~ /^cc/)) { + defined $qualifier && $qualifier !~ /^c(?:[hlbc]|hR)$/)) { $bad_specifier = $specifier; last; } @@ -6994,21 +7073,20 @@ sub process { # } # } # } - # strcpy uses that should likely be strscpy - if ($line =~ /\bstrcpy\s*\(/) { + if ($line =~ /\bstrcpy\s*\(/ && !is_userspace($realfile)) { WARN("STRCPY", "Prefer strscpy over strcpy - see: https://github.com/KSPP/linux/issues/88\n" . $herecurr); } # strlcpy uses that should likely be strscpy - if ($line =~ /\bstrlcpy\s*\(/) { + if ($line =~ /\bstrlcpy\s*\(/ && !is_userspace($realfile)) { WARN("STRLCPY", "Prefer strscpy over strlcpy - see: https://github.com/KSPP/linux/issues/89\n" . $herecurr); } # strncpy uses that should likely be strscpy or strscpy_pad - if ($line =~ /\bstrncpy\s*\(/) { + if ($line =~ /\bstrncpy\s*\(/ && !is_userspace($realfile)) { WARN("STRNCPY", "Prefer strscpy, strscpy_pad, or __nonstring over strncpy - see: https://github.com/KSPP/linux/issues/90\n" . $herecurr); } @@ -7219,17 +7297,42 @@ sub process { "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } -# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc +# check for (kv|k)[mz]alloc that could be kmalloc_obj/kvmalloc_obj/kzalloc_obj/kvzalloc_obj + if ($perl_version_ok && + defined $stat && + $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*,/) { + my $oldfunc = $3; + my $a1 = $4; + my $newfunc = "kmalloc_obj"; + $newfunc = "kvmalloc_obj" if ($oldfunc eq "kvmalloc"); + $newfunc = "kvzalloc_obj" if ($oldfunc eq "kvzalloc"); + $newfunc = "kzalloc_obj" if ($oldfunc eq "kzalloc"); + + if ($a1 =~ s/^sizeof\s*\S\(?([^\)]*)\)?$/$1/) { + my $cnt = statement_rawlines($stat); + my $herectx = get_stat_here($linenr, $cnt, $here); + + if (WARN("ALLOC_WITH_SIZEOF", + "Prefer $newfunc over $oldfunc with sizeof\n" . $herectx) && + $cnt == 1 && + $fix) { + $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*,/$1 = $newfunc($a1,/; + } + } + } + + +# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_objs/kvmalloc_objs/kzalloc_objs/kvzalloc_objs if ($perl_version_ok && defined $stat && $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) { my $oldfunc = $3; my $a1 = $4; my $a2 = $10; - my $newfunc = "kmalloc_array"; - $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc"); - $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc"); - $newfunc = "kcalloc" if ($oldfunc eq "kzalloc"); + my $newfunc = "kmalloc_objs"; + $newfunc = "kvmalloc_objs" if ($oldfunc eq "kvmalloc"); + $newfunc = "kvzalloc_objs" if ($oldfunc eq "kvzalloc"); + $newfunc = "kzalloc_objs" if ($oldfunc eq "kzalloc"); my $r1 = $a1; my $r2 = $a2; if ($a1 =~ /^sizeof\s*\S/) { @@ -7245,7 +7348,9 @@ sub process { "Prefer $newfunc over $oldfunc with multiply\n" . $herectx) && $cnt == 1 && $fix) { - $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; + my $sized = trim($r2); + $sized =~ s/^sizeof\s*\S\(?([^\)]*)\)?$/$1/; + $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . $sized . ', ' . trim($r1)/e; } } } @@ -7415,10 +7520,10 @@ sub process { } # check for various structs that are normally const (ops, kgdb, device_tree) -# and avoid what seem like struct definitions 'struct foo {' +# and avoid what seem like struct definitions 'struct foo {' or forward declarations 'struct foo;' if (defined($const_structs) && $line !~ /\bconst\b/ && - $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) { + $line =~ /\bstruct\s+($const_structs)\b(?!\s*[\{;])/) { WARN("CONST_STRUCT", "struct $1 should normally be const\n" . $herecurr); } @@ -7668,6 +7773,37 @@ sub process { WARN("DUPLICATED_SYSCTL_CONST", "duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr); } + +# Check that *_device_id tables have sentinel entries. + if (defined $stat && $line =~ /struct\s+$dev_id_types\s+\w+\s*\[\s*\]\s*=\s*\{/) { + my $stripped = $stat; + + # Strip diff line prefixes. + $stripped =~ s/(^|\n)./$1/g; + # Line continuations. + $stripped =~ s/\\\n/\n/g; + # Strip whitespace, empty strings, zeroes, and commas. + $stripped =~ s/""//g; + $stripped =~ s/0x0//g; + $stripped =~ s/[\s$;,0]//g; + # Strip field assignments. + $stripped =~ s/\.$Ident=//g; + + if (!(substr($stripped, -4) eq "{}};" || + substr($stripped, -6) eq "{{}}};" || + $stripped =~ /ISAPNP_DEVICE_SINGLE_END}};$/ || + $stripped =~ /ISAPNP_CARD_END}};$/ || + $stripped =~ /NULL};$/ || + $stripped =~ /PCMCIA_DEVICE_NULL};$/)) { + ERROR("MISSING_SENTINEL", "missing sentinel in ID array\n" . "$here\n$stat\n"); + } + } + +# check for uninitialized pointers with __free attribute + while ($line =~ /\*\s*($Ident)\s+__free\s*\(\s*$Ident\s*\)\s*[,;]/g) { + ERROR("UNINITIALIZED_PTR_WITH_FREE", + "pointer '$1' with __free attribute should be initialized\n" . $herecurr); + } } # If we have no input at all, then there is nothing to report on |