From 85b0ee18bbf82cb3e1880c718749149c6ed61058 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:51:44 -0700 Subject: checkpatch: see if modified files are marked obsolete in MAINTAINERS Use get_maintainer to check the status of individual files. If "obsolete", suggest leaving the files alone. Link: http://lkml.kernel.org/r/7ceaa510dc9d2df05ec4b456baed7bb1415550b3.1471889575.git.joe@perches.com Signed-off-by: Joe Perches Cc: SF Markus Elfring Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 206a6b346a8d..34b445574011 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -704,6 +704,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 $filename 2>&1`; + + return $status =~ /obsolete/i; +} + my $camelcase_seeded = 0; sub seed_camelcase_includes { return if ($camelcase_seeded); @@ -2289,6 +2299,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 { -- cgit v1.2.3 From f90774e1fd2700de4a6e0d62866d34a26c544bd0 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:51:47 -0700 Subject: checkpatch: look for symbolic permissions and suggest octal instead S_ uses should be avoided where octal is more intelligible. Linus didst say: : It's *much* easier to parse and understand the octal numbers, while the : symbolic macro names are just random line noise and hard as hell to : understand. You really have to think about it. : : So we should rather go the other way: convert existing bad symbolic : permission bit macro use to just use the octal numbers. : : The symbolic names are good for the *other* bits (ie sticky bit, and the : inode mode _type_ numbers etc), but for the permission bits, the symbolic : names are just insane crap. Nobody sane should ever use them. Not in the : kernel, not in user space. (http://lkml.kernel.org/r/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com) Link: http://lkml.kernel.org/r/7232ef011d05a92f4caa86a5e9830d87966a2eaf.1470180926.git.joe@perches.com Signed-off-by: Joe Perches Cc: Linus Torvalds Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 49 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 6 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 34b445574011..1c82b01d581a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -541,6 +541,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| @@ -6003,20 +6029,31 @@ sub process { $arg_pos--; $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; } - my $test = "\\b$func\\s*\\(${skip_args}([\\d]+)\\s*[,\\)]"; + my $test = "\\b$func\\s*\\(${skip_args}($FuncArg(?:\\|\\s*$FuncArg)*)\\s*[,\\)]"; if ($line =~ /$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)) { + } + if ($val =~ /^$Octal$/ && (oct($val) & 02)) { ERROR("EXPORTED_WORLD_WRITABLE", "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } + if ($val =~ /\b$mode_perms_string_search\b/) { + my $to = 0; + while ($val =~ /\b($mode_perms_string_search)\b(?:\s*\|\s*)?\s*/g) { + $to |= $mode_permission_string_types{$1}; + } + my $new = sprintf("%04o", $to); + if (WARN("SYMBOLIC_PERMS", + "Symbolic permissions are not preferred. Consider using octal permissions $new.\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$val\E/$new/; + } + } } } } -- cgit v1.2.3 From 08eb9b8016b18f53fa8900f3d1c9359f9edeb724 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:51:50 -0700 Subject: checkpatch: test multiple line block comment alignment Warn when block comments are not aligned on the * /* * block comment, no warning */ /* * block comment, emit warning */ Link: http://lkml.kernel.org/r/edb57bd330adfe024b95ec2a807d4aa7f0c8b112.1472261299.git.joe@perches.com Signed-off-by: Joe Perches Reported-by: Sudip Mukherjee Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1c82b01d581a..894f8c616573 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2979,6 +2979,25 @@ sub process { "Block comments use a trailing */ on a separate line\n" . $herecurr); } +# Block comment * alignment + if ($prevline =~ /$;[ \t]*$/ && #ends in comment + (($prevrawline =~ /^\+.*?\/\*/ && #starting /* + $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */ + $prevrawline =~ /^\+[ \t]*\*/) && #starting * + $rawline =~ /^\+[ \t]*\*/) { #rawline * + $prevrawline =~ m@^\+([ \t]*/?)\*@; + my $oldindent = expand_tabs($1); + $rawline =~ m@^\+([ \t]*)\*@; + my $newindent = $1; + my $test_comment = '^\\+' . "$;" x (length($newindent) + 1); + $newindent = expand_tabs($newindent); + if ($line =~ /$test_comment/ && + 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*$/ && -- cgit v1.2.3 From f333195d41e15e7b105ca270c7662204a1a9d0f5 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:51:53 -0700 Subject: checkpatch: don't test for prefer ether_addr_ < sigh > Comment these tests out. These are just too enticing to people that don't verify that both source and dest addresses really must be __aligned(2). It helps make Dan Carpenter happy too. Link: http://lkml.kernel.org/r/dc32ec66d24647f4cdf824c8dfbbc59aa7ce7b7d.1472665676.git.joe@perches.com Signed-off-by: Joe Perches Cc: Dan Carpenter Cc: Greg Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 70 +++++++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 35 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 894f8c616573..37d8b91ac198 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5554,46 +5554,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 && -- cgit v1.2.3 From bf1fa1dae68e1b58b7b7fd61bde654d37da1de57 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:51:56 -0700 Subject: checkpatch: externalize the structs that should be const Make it easier to add new structs that should be const. Link: http://lkml.kernel.org/r/e5a8da43e7c11525bafbda1ca69a8323614dd942.1472664220.git.joe@perches.com Signed-off-by: Joe Perches Cc: Julia Lawall Cc: Kees Cook Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 64 +++++++++++++++------------------------- scripts/const_structs.checkpatch | 39 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 40 deletions(-) create mode 100644 scripts/const_structs.checkpatch (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 37d8b91ac198..4ecb66c595f4 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; @@ -624,6 +625,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)"; @@ -5912,46 +5936,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", diff --git a/scripts/const_structs.checkpatch b/scripts/const_structs.checkpatch new file mode 100644 index 000000000000..1b54425f6c89 --- /dev/null +++ b/scripts/const_structs.checkpatch @@ -0,0 +1,39 @@ +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 -- cgit v1.2.3 From 0616efa45af493a259d25c1716102173f10bd789 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:52:02 -0700 Subject: checkpatch: speed up checking for filenames in sections marked obsolete Adding -f to the get_maintainer.pl invocation means git isn't invoked by get_maintainer.pl for known filenames. This reduces the overall time to run checkpatch. Link: http://lkml.kernel.org/r/22991e3a295aeb399b43af0478b6e5809106ccee.1472684066.git.joe@perches.com Signed-off-by: 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 4ecb66c595f4..e44dc0c75d54 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -759,7 +759,7 @@ sub is_maintained_obsolete { return 0 if (!(-e "$root/scripts/get_maintainer.pl")); - my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback $filename 2>&1`; + my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback -f $filename 2>&1`; return $status =~ /obsolete/i; } -- cgit v1.2.3 From af207524a49c3d70af0a9196072ab3b6d690fcf7 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:52:05 -0700 Subject: checkpatch: improve the block comment * alignment test An "uninitialized value" is emitted when a block comment starts on the same line as a statement. Fix this and make the test use a little fewer cpu cycles too. Link: http://lkml.kernel.org/r/3c9993320c2182d37f53ac540878cfef59c3f62d.1473365956.git.joe@perches.com Signed-off-by: Joe Perches Reported-by: Charlemagne Lasse Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e44dc0c75d54..3fa2b2e973eb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3005,18 +3005,23 @@ sub process { # Block comment * alignment if ($prevline =~ /$;[ \t]*$/ && #ends in comment - (($prevrawline =~ /^\+.*?\/\*/ && #starting /* + $line =~ /^\+[ \t]*$;/ && #leading comment + $rawline =~ /^\+[ \t]*\*/ && #leading * + (($prevrawline =~ /^\+.*?\/\*/ && #leading /* $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */ - $prevrawline =~ /^\+[ \t]*\*/) && #starting * - $rawline =~ /^\+[ \t]*\*/) { #rawline * + $prevrawline =~ /^\+[ \t]*\*/)) { #leading * + my $oldindent; $prevrawline =~ m@^\+([ \t]*/?)\*@; - my $oldindent = expand_tabs($1); + if (defined($1)) { + $oldindent = expand_tabs($1); + } else { + $prevrawline =~ m@^\+(.*/?)\*@; + $oldindent = expand_tabs($1); + } $rawline =~ m@^\+([ \t]*)\*@; my $newindent = $1; - my $test_comment = '^\\+' . "$;" x (length($newindent) + 1); $newindent = expand_tabs($newindent); - if ($line =~ /$test_comment/ && - length($oldindent) ne length($newindent)) { + if (length($oldindent) ne length($newindent)) { WARN("BLOCK_COMMENT_STYLE", "Block comments should align the * on each line\n" . $hereprev); } -- cgit v1.2.3 From f59b64bffe163431b4ec61f3b00c1aeb846948f3 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:52:08 -0700 Subject: checkpatch: add --strict test for macro argument reuse If a macro argument is used multiple times in the macro definition, the macro argument may have an unexpected side-effect. Add a test (MACRO_ARG_REUSE) for that condition which is only emitted with command-line option --strict. Link: http://lkml.kernel.org/r/b6d67a87cafcafd15499e91780dc63b15dec0aa0.1473744906.git.joe@perches.com Signed-off-by: Joe Perches Cc: Andy Whitcroft Cc: Julia Lawall Cc: Dan Carpenter Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3fa2b2e973eb..f135f8e4d069 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4753,7 +4753,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; @@ -4789,6 +4799,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(); @@ -4804,13 +4823,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", @@ -4819,6 +4831,21 @@ sub process { ERROR("COMPLEX_MACRO", "Macros with complex values should be enclosed in parentheses\n" . "$herectx"); } + + } +# check if any macro arguments are reused (ignore '...' and 'type') + foreach my $arg (@def_args) { + next if ($arg =~ /\.\.\./); + next if ($arg =~ /^type$/); + 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 for macros with flow control, but without ## concatenation -- cgit v1.2.3 From 9192d41a3f0b98844860f1de5b82f729bbe07c61 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:52:11 -0700 Subject: checkpatch: add --strict test for precedence challenged macro arguments Add a test for macro arguents that have a non-comma leading or trailing operator where the argument isn't parenthesized to avoid possible precedence issues. Link: http://lkml.kernel.org/r/47715508972f8d786f435e583ff881dbeee3a114.1473745855.git.joe@perches.com Signed-off-by: Joe Perches Cc: Andy Whitcroft Cc: Julia Lawall Cc: Dan Carpenter Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f135f8e4d069..ea1a7adce406 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4836,7 +4836,7 @@ sub process { # check if any macro arguments are reused (ignore '...' and 'type') foreach my $arg (@def_args) { next if ($arg =~ /\.\.\./); - next if ($arg =~ /^type$/); + 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; @@ -4845,6 +4845,13 @@ sub process { 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"); } } -- cgit v1.2.3 From 5207649b7b1d2b1e1f8c97df91debfaa8de37669 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:52:14 -0700 Subject: checkpatch: improve MACRO_ARG_PRECEDENCE test It is possible for a multiple line macro definition to have a false positive report when an argument is used on a line after a continuation \. This line might have a leading '+' as the initial character that could be confused by checkpatch as an operator. Avoid the leading character on multiple line macro definitions. Link: http://lkml.kernel.org/r/60229d13399f9b6509db5a32e30d4c16951a60cd.1473836073.git.joe@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index ea1a7adce406..0ef3d837f2aa 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4833,13 +4833,31 @@ sub process { } } + + # 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/\#+\s*$arg\b//g; $tmp =~ s/\b$arg\s*\#\#//g; my $use_cnt = $tmp =~ s/\b$arg\b//g; if ($use_cnt > 1) { -- cgit v1.2.3 From ca0d8929e75ab1f860f61208d46955f280a1b05e Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:52:16 -0700 Subject: checkpatch: add warning for unnamed function definition arguments Function definitions without identifiers like int foo(int) are not preferred. Emit a warning when they occur. Link: http://lkml.kernel.org/r/94fe6378504745991b650f48fc92bb4648f25706.1474925354.git.joe@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 0ef3d837f2aa..3373c65fef1c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5794,6 +5794,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; -- cgit v1.2.3 From 459cf0ae5d6600592a539bc861c84e14ed1a5bcb Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 11 Oct 2016 13:52:19 -0700 Subject: checkpatch: improve the octal permissions tests The function calls with octal permissions commonly span multiple lines. The current test is line oriented and fails to find some matches. Make the test use the $stat variable instead of the $line variable to span multiple lines. Also add a few functions to the known functions with permissions list. Move the SYMBOLIC_PERMS test to a separate section to find all the S_ permissions in any form not just those that have specific function names. This can now find and fix permissions uses like: .mode = S_ | S_; Link: http://lkml.kernel.org/r/b51bab60530912aae4ac420119d465c5b206f19f.1475030406.git.joe@perches.com Signed-off-by: Joe Perches Tested-by: Ramiro Oliveira Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 60 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 16 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3373c65fef1c..a8368d1c4348 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -524,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 @@ -6092,45 +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}($FuncArg(?:\\|\\s*$FuncArg)*)\\s*[,\\)]"; - if ($line =~ /$test/) { + if ($stat =~ /$test/) { my $val = $1; $val = $6 if ($skip_args ne ""); 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); + "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); - } - if ($val =~ /\b$mode_perms_string_search\b/) { - my $to = 0; - while ($val =~ /\b($mode_perms_string_search)\b(?:\s*\|\s*)?\s*/g) { - $to |= $mode_permission_string_types{$1}; - } - my $new = sprintf("%04o", $to); - if (WARN("SYMBOLIC_PERMS", - "Symbolic permissions are not preferred. Consider using octal permissions $new.\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\Q$val\E/$new/; - } + "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . "$here\n" . $stat_real); } } } } +# check for uses of S_ 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); -- cgit v1.2.3