diff options
Diffstat (limited to 'scripts/checkpatch.pl')
| -rwxr-xr-x | scripts/checkpatch.pl | 347 |
1 files changed, 245 insertions, 102 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4de3cc42fc50..a8368d1c4348 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -54,6 +54,7 @@ my $min_conf_desc_length = 4; my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; +my $conststructsfile = "$D/const_structs.checkpatch"; my $color = 1; my $allow_c99_comments = 1; @@ -523,7 +524,11 @@ our @mode_permission_funcs = ( ["module_param_array_named", 5], ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2], ["proc_create(?:_data|)", 2], - ["(?:CLASS|DEVICE|SENSOR)_ATTR", 2], + ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2], + ["IIO_DEV_ATTR_[A-Z_]+", 1], + ["SENSOR_(?:DEVICE_|)ATTR_2", 2], + ["SENSOR_TEMPLATE(?:_2|)", 3], + ["__ATTR", 2], ); #Create a search pattern for all these functions to speed up a loop below @@ -541,6 +546,32 @@ our $mode_perms_world_writable = qr{ 0[0-7][0-7][2367] }x; +our %mode_permission_string_types = ( + "S_IRWXU" => 0700, + "S_IRUSR" => 0400, + "S_IWUSR" => 0200, + "S_IXUSR" => 0100, + "S_IRWXG" => 0070, + "S_IRGRP" => 0040, + "S_IWGRP" => 0020, + "S_IXGRP" => 0010, + "S_IRWXO" => 0007, + "S_IROTH" => 0004, + "S_IWOTH" => 0002, + "S_IXOTH" => 0001, + "S_IRWXUGO" => 0777, + "S_IRUGO" => 0444, + "S_IWUGO" => 0222, + "S_IXUGO" => 0111, +); + +#Create a search pattern for all these strings to speed up a loop below +our $mode_perms_string_search = ""; +foreach my $entry (keys %mode_permission_string_types) { + $mode_perms_string_search .= '|' if ($mode_perms_string_search ne ""); + $mode_perms_string_search .= $entry; +} + our $allowed_asm_includes = qr{(?x: irq| memory| @@ -598,6 +629,29 @@ if ($codespell) { $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix; +my $const_structs = ""; +if (open(my $conststructs, '<', $conststructsfile)) { + while (<$conststructs>) { + my $line = $_; + + $line =~ s/\s*\n?$//g; + $line =~ s/^\s*//g; + + next if ($line =~ m/^\s*#/); + next if ($line =~ m/^\s*$/); + if ($line =~ /\s/) { + print("$conststructsfile: '$line' invalid - ignored\n"); + next; + } + + $const_structs .= '|' if ($const_structs ne ""); + $const_structs .= $line; + } + close($conststructsfile); +} else { + warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; +} + sub build_types { my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; my $all = "(?x: \n" . join("|\n ", (@typeList, @typeListFile)) . "\n)"; @@ -704,6 +758,16 @@ sub seed_camelcase_file { } } +sub is_maintained_obsolete { + my ($filename) = @_; + + return 0 if (!(-e "$root/scripts/get_maintainer.pl")); + + my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback -f $filename 2>&1`; + + return $status =~ /obsolete/i; +} + my $camelcase_seeded = 0; sub seed_camelcase_includes { return if ($camelcase_seeded); @@ -2289,6 +2353,10 @@ sub process { } if ($found_file) { + if (is_maintained_obsolete($realfile)) { + WARN("OBSOLETE", + "$realfile is marked as 'obsolete' in the MAINTAINERS hierarchy. No unnecessary modifications please.\n"); + } if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) { $check = 1; } else { @@ -2939,6 +3007,30 @@ sub process { "Block comments use a trailing */ on a separate line\n" . $herecurr); } +# Block comment * alignment + if ($prevline =~ /$;[ \t]*$/ && #ends in comment + $line =~ /^\+[ \t]*$;/ && #leading comment + $rawline =~ /^\+[ \t]*\*/ && #leading * + (($prevrawline =~ /^\+.*?\/\*/ && #leading /* + $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */ + $prevrawline =~ /^\+[ \t]*\*/)) { #leading * + my $oldindent; + $prevrawline =~ m@^\+([ \t]*/?)\*@; + if (defined($1)) { + $oldindent = expand_tabs($1); + } else { + $prevrawline =~ m@^\+(.*/?)\*@; + $oldindent = expand_tabs($1); + } + $rawline =~ m@^\+([ \t]*)\*@; + my $newindent = $1; + $newindent = expand_tabs($newindent); + if (length($oldindent) ne length($newindent)) { + WARN("BLOCK_COMMENT_STYLE", + "Block comments should align the * on each line\n" . $hereprev); + } + } + # check for missing blank lines after struct/union declarations # with exceptions for various attributes and macros if ($prevline =~ /^[\+ ]};?\s*$/ && @@ -3570,15 +3662,6 @@ sub process { } } -# check for uses of DEFINE_PCI_DEVICE_TABLE - if ($line =~ /\bDEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=/) { - if (WARN("DEFINE_PCI_DEVICE_TABLE", - "Prefer struct pci_device_id over deprecated DEFINE_PCI_DEVICE_TABLE\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b(?:static\s+|)DEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=\s*/static const struct pci_device_id $1\[\] = /; - } - } - # check for new typedefs, only function parameters and sparse annotations # make sense. if ($line =~ /\btypedef\s/ && @@ -4674,7 +4757,17 @@ sub process { $has_flow_statement = 1 if ($ctx =~ /\b(goto|return)\b/); $has_arg_concat = 1 if ($ctx =~ /\#\#/ && $ctx !~ /\#\#\s*(?:__VA_ARGS__|args)\b/); - $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//; + $dstat =~ s/^.\s*\#\s*define\s+$Ident(\([^\)]*\))?\s*//; + my $define_args = $1; + my $define_stmt = $dstat; + my @def_args = (); + + if (defined $define_args && $define_args ne "") { + $define_args = substr($define_args, 1, length($define_args) - 2); + $define_args =~ s/\s*//g; + @def_args = split(",", $define_args); + } + $dstat =~ s/$;//g; $dstat =~ s/\\\n.//g; $dstat =~ s/^\s*//s; @@ -4710,6 +4803,15 @@ sub process { ^\[ }x; #print "REST<$rest> dstat<$dstat> ctx<$ctx>\n"; + + $ctx =~ s/\n*$//; + my $herectx = $here . "\n"; + my $stmt_cnt = statement_rawlines($ctx); + + for (my $n = 0; $n < $stmt_cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + if ($dstat ne '' && $dstat !~ /^(?:$Ident|-?$Constant),$/ && # 10, // foo(), $dstat !~ /^(?:$Ident|-?$Constant);$/ && # foo(); @@ -4725,13 +4827,6 @@ sub process { $dstat !~ /^\(\{/ && # ({... $ctx !~ /^.\s*#\s*define\s+TRACE_(?:SYSTEM|INCLUDE_FILE|INCLUDE_PATH)\b/) { - $ctx =~ s/\n*$//; - my $herectx = $here . "\n"; - my $cnt = statement_rawlines($ctx); - - for (my $n = 0; $n < $cnt; $n++) { - $herectx .= raw_line($linenr, $n) . "\n"; - } if ($dstat =~ /;/) { ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", @@ -4740,6 +4835,46 @@ sub process { ERROR("COMPLEX_MACRO", "Macros with complex values should be enclosed in parentheses\n" . "$herectx"); } + + } + + # Make $define_stmt single line, comment-free, etc + my @stmt_array = split('\n', $define_stmt); + my $first = 1; + $define_stmt = ""; + foreach my $l (@stmt_array) { + $l =~ s/\\$//; + if ($first) { + $define_stmt = $l; + $first = 0; + } elsif ($l =~ /^[\+ ]/) { + $define_stmt .= substr($l, 1); + } + } + $define_stmt =~ s/$;//g; + $define_stmt =~ s/\s+/ /g; + $define_stmt = trim($define_stmt); + +# check if any macro arguments are reused (ignore '...' and 'type') + foreach my $arg (@def_args) { + next if ($arg =~ /\.\.\./); + next if ($arg =~ /^type$/i); + my $tmp = $define_stmt; + $tmp =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; + $tmp =~ s/\#+\s*$arg\b//g; + $tmp =~ s/\b$arg\s*\#\#//g; + my $use_cnt = $tmp =~ s/\b$arg\b//g; + if ($use_cnt > 1) { + CHK("MACRO_ARG_REUSE", + "Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx"); + } +# check if any macro arguments may have other precedence issues + if ($define_stmt =~ m/($Operators)?\s*\b$arg\b\s*($Operators)?/m && + ((defined($1) && $1 ne ',') || + (defined($2) && $2 ne ','))) { + CHK("MACRO_ARG_PRECEDENCE", + "Macro argument '$arg' may be better as '($arg)' to avoid precedence issues\n" . "$herectx"); + } } # check for macros with flow control, but without ## concatenation @@ -5504,46 +5639,46 @@ sub process { } # Check for memcpy(foo, bar, ETH_ALEN) that could be ether_addr_copy(foo, bar) - if ($^V && $^V ge 5.10.0 && - defined $stat && - $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { - if (WARN("PREFER_ETHER_ADDR_COPY", - "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . "$here\n$stat\n") && - $fix) { - $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; - } - } +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# if (WARN("PREFER_ETHER_ADDR_COPY", +# "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; +# } +# } # Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar) - if ($^V && $^V ge 5.10.0 && - defined $stat && - $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { - WARN("PREFER_ETHER_ADDR_EQUAL", - "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") - } +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# WARN("PREFER_ETHER_ADDR_EQUAL", +# "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") +# } # check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr # check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr - if ($^V && $^V ge 5.10.0 && - defined $stat && - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { - - my $ms_val = $7; - - if ($ms_val =~ /^(?:0x|)0+$/i) { - if (WARN("PREFER_ETH_ZERO_ADDR", - "Prefer eth_zero_addr over memset()\n" . "$here\n$stat\n") && - $fix) { - $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($2)/; - } - } elsif ($ms_val =~ /^(?:0xff|255)$/i) { - if (WARN("PREFER_ETH_BROADCAST_ADDR", - "Prefer eth_broadcast_addr() over memset()\n" . "$here\n$stat\n") && - $fix) { - $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($2)/; - } - } - } +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# +# my $ms_val = $7; +# +# if ($ms_val =~ /^(?:0x|)0+$/i) { +# if (WARN("PREFER_ETH_ZERO_ADDR", +# "Prefer eth_zero_addr over memset()\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($2)/; +# } +# } elsif ($ms_val =~ /^(?:0xff|255)$/i) { +# if (WARN("PREFER_ETH_BROADCAST_ADDR", +# "Prefer eth_broadcast_addr() over memset()\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($2)/; +# } +# } +# } # typecasts on min/max could be min_t/max_t if ($^V && $^V ge 5.10.0 && @@ -5663,6 +5798,19 @@ sub process { "externs should be avoided in .c files\n" . $herecurr); } + if ($realfile =~ /\.[ch]$/ && defined $stat && + $stat =~ /^.\s*(?:extern\s+)?$Type\s*$Ident\s*\(\s*([^{]+)\s*\)\s*;/s && + $1 ne "void") { + my $args = trim($1); + while ($args =~ m/\s*($Type\s*(?:$Ident|\(\s*\*\s*$Ident?\s*\)\s*$balanced_parens)?)/g) { + my $arg = trim($1); + if ($arg =~ /^$Type$/ && $arg !~ /enum\s+$Ident$/) { + WARN("FUNCTION_ARGUMENTS", + "function definition argument '$arg' should also have an identifier name\n" . $herecurr); + } + } + } + # checks for new __setup's if ($rawline =~ /\b__setup\("([^"]*)"/) { my $name = $1; @@ -5862,46 +6010,6 @@ sub process { } # check for various structs that are normally const (ops, kgdb, device_tree) - my $const_structs = qr{ - acpi_dock_ops| - address_space_operations| - backlight_ops| - block_device_operations| - dentry_operations| - dev_pm_ops| - dma_map_ops| - extent_io_ops| - file_lock_operations| - file_operations| - hv_ops| - ide_dma_ops| - intel_dvo_dev_ops| - item_operations| - iwl_ops| - kgdb_arch| - kgdb_io| - kset_uevent_ops| - lock_manager_operations| - microcode_ops| - mtrr_ops| - neigh_ops| - nlmsvc_binding| - of_device_id| - pci_raw_ops| - pipe_buf_operations| - platform_hibernation_ops| - platform_suspend_ops| - proto_ops| - rpc_pipe_ops| - seq_operations| - snd_ac97_build_ops| - soc_pcmcia_socket_ops| - stacktrace_ops| - sysfs_ops| - tty_operations| - uart_ops| - usb_mon_operations| - wd_ops}x; if ($line !~ /\bconst\b/ && $line =~ /\bstruct\s+($const_structs)\b/) { WARN("CONST_STRUCT", @@ -5988,34 +6096,69 @@ sub process { # Mode permission misuses where it seems decimal should be octal # This uses a shortcut match to avoid unnecessary uses of a slow foreach loop if ($^V && $^V ge 5.10.0 && + defined $stat && $line =~ /$mode_perms_search/) { foreach my $entry (@mode_permission_funcs) { my $func = $entry->[0]; my $arg_pos = $entry->[1]; + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } + my $skip_args = ""; if ($arg_pos > 1) { $arg_pos--; $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; } - my $test = "\\b$func\\s*\\(${skip_args}([\\d]+)\\s*[,\\)]"; - if ($line =~ /$test/) { + my $test = "\\b$func\\s*\\(${skip_args}($FuncArg(?:\\|\\s*$FuncArg)*)\\s*[,\\)]"; + if ($stat =~ /$test/) { my $val = $1; $val = $6 if ($skip_args ne ""); - - if ($val !~ /^0$/ && - (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || - length($val) ne 4)) { + if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || + ($val =~ /^$Octal$/ && length($val) ne 4)) { ERROR("NON_OCTAL_PERMISSIONS", - "Use 4 digit octal (0777) not decimal permissions\n" . $herecurr); - } elsif ($val =~ /^$Octal$/ && (oct($val) & 02)) { + "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real); + } + if ($val =~ /^$Octal$/ && (oct($val) & 02)) { ERROR("EXPORTED_WORLD_WRITABLE", - "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); + "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . "$here\n" . $stat_real); } } } } +# check for uses of S_<PERMS> that could be octal for readability + if ($line =~ /\b$mode_perms_string_search\b/) { + my $val = ""; + my $oval = ""; + my $to = 0; + my $curpos = 0; + my $lastpos = 0; + while ($line =~ /\b(($mode_perms_string_search)\b(?:\s*\|\s*)?\s*)/g) { + $curpos = pos($line); + my $match = $2; + my $omatch = $1; + last if ($lastpos > 0 && ($curpos - length($omatch) != $lastpos)); + $lastpos = $curpos; + $to |= $mode_permission_string_types{$match}; + $val .= '\s*\|\s*' if ($val ne ""); + $val .= $match; + $oval .= $omatch; + } + $oval =~ s/^\s*\|\s*//; + $oval =~ s/\s*\|\s*$//; + my $octal = sprintf("%04o", $to); + if (WARN("SYMBOLIC_PERMS", + "Symbolic permissions '$oval' are not preferred. Consider using octal permissions '$octal'.\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/$val/$octal/; + } + } + # validate content of MODULE_LICENSE against list from include/linux/module.h if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) { my $extracted_string = get_quoted_string($line, $rawline); |