From 25bdda2bd68ae3008759f26058f6f9b9bb2b1cc5 Mon Sep 17 00:00:00 2001 From: Miles Chen Date: Fri, 17 Nov 2017 15:28:34 -0800 Subject: checkpatch: support function pointers for unnamed function definition arguments Current unnamed function definition argument does not include function pointer cases and it reports something like: WARNING: function definition argument 'void' should also have an identifier name +unsigned int (*dummy)(void); Support function pointers for unnamed function arguments Link: http://lkml.kernel.org/r/1505389925-31087-1-git-send-email-miles.chen@mediatek.com Signed-off-by: Miles Chen Acked-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 8b80bac055e4..aacbe918027b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5957,7 +5957,7 @@ sub process { # check for function declarations that have arguments without identifier names if (defined $stat && - $stat =~ /^.\s*(?:extern\s+)?$Type\s*$Ident\s*\(\s*([^{]+)\s*\)\s*;/s && + $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:$Ident|\(\s*\*\s*$Ident\s*\))\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) { -- cgit v1.2.3 From 258f79d5a1e49271f5aff38e6c1baeeaad0d82aa Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 17 Nov 2017 15:28:38 -0800 Subject: scripts/checkpatch.pl: avoid false warning missing break void foo(int a) switch (a) { case 'h': fun1(); exit(1); default: } creates a warning "Possible switch case/default not preceded by break or fallthrough comment". exit( should be treated like return. Link: http://lkml.kernel.org/r/20170910154618.25819-1-xypron.glpk@gmx.de Signed-off-by: Heinrich Schuchardt Acked-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 aacbe918027b..8dce8a8d9ed0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6109,7 +6109,7 @@ sub process { next if ($fline =~ /^.[\s$;]*$/); $has_statement = 1; $count++; - $has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|return\b|goto\b|continue\b)/); + $has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|exit\s*\(\b|return\b|goto\b|continue\b)/); } if (!$has_break && $has_statement) { WARN("MISSING_BREAK", -- cgit v1.2.3 From eeef5733e30e736926c762fa3336c4dd5702bcdf Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 17 Nov 2017 15:28:41 -0800 Subject: checkpatch: printks always need a KERN_ There was code in checkpatch that allowed continuation printks to be used without KERN_CONT. Remove the continuation check and always require a KERN_. Link: http://lkml.kernel.org/r/61980ef41d5b9b6543da1c49055042e0ab74d308.1507047008.git.joe@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 8dce8a8d9ed0..2a8c6c3c1bdb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3829,28 +3829,10 @@ sub process { "Prefer printk_ratelimited or pr__ratelimited to printk_ratelimit\n" . $herecurr); } -# printk should use KERN_* levels. Note that follow on printk's on the -# same line do not need a level, so we use the current block context -# to try and find and validate the current printk. In summary the current -# printk includes all preceding printk's which have no newline on the end. -# we assume the first bad printk is the one to report. - if ($line =~ /\bprintk\((?!KERN_)\s*"/) { - my $ok = 0; - for (my $ln = $linenr - 1; $ln >= $first_line; $ln--) { - #print "CHECK<$lines[$ln - 1]\n"; - # we have a preceding printk if it ends - # with "\n" ignore it, else it is to blame - if ($lines[$ln - 1] =~ m{\bprintk\(}) { - if ($rawlines[$ln - 1] !~ m{\\n"}) { - $ok = 1; - } - last; - } - } - if ($ok == 0) { - WARN("PRINTK_WITHOUT_KERN_LEVEL", - "printk() should include KERN_ facility level\n" . $herecurr); - } +# printk should use KERN_* levels + if ($line =~ /\bprintk\s*\(\s*(?!KERN_[A-Z]+\b)/) { + WARN("PRINTK_WITHOUT_KERN_LEVEL", + "printk() should include KERN_ facility level\n" . $herecurr); } if ($line =~ /\bprintk\s*\(\s*KERN_([A-Z]+)/) { -- cgit v1.2.3 From cc147506bef96f31af799afe94d0c0e81161ee6c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 17 Nov 2017 15:28:44 -0800 Subject: checkpatch: allow DEFINE_PER_CPU definitions to exceed line length Some of the definitions are very long and can't be split into multiple lines because ctags is limited. Exempt these lines from the line length checks. See commit 25528213fe9f ("tags: Fix DEFINE_PER_CPU expansions") for more details. Link: http://lkml.kernel.org/r/1508170320.6530.15.camel@perches.com Signed-off-by: Joe Perches Acked-by: Mark Rutland Cc: Daniel Lezcano Cc: Marc Zyngier Cc: Thomas Gleixner Cc: Ard Biesheuvel Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2a8c6c3c1bdb..5fa0f5467d99 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2900,8 +2900,9 @@ sub process { $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) { $msg_type = ""; - # EFI_GUID is another special case - } elsif ($line =~ /^\+.*\bEFI_GUID\s*\(/) { + # More special cases + } elsif ($line =~ /^\+.*\bEFI_GUID\s*\(/ || + $line =~ /^\+\s*(?:\w+)?\s*DEFINE_PER_CPU/) { $msg_type = ""; # Otherwise set the alternate message types -- cgit v1.2.3 From 87bd499af5cd663c150032cca4bac822a729263b Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 17 Nov 2017 15:28:48 -0800 Subject: checkpatch: add TP_printk to list of logging functions So the line length check can be bypassed by its callers. Link: http://lkml.kernel.org/r/7de542c08a6e79f2ebe7c1416c9f403c23fdcc09.1508282823.git.joe@perches.com Signed-off-by: Joe Perches Reported-by: Song Liu Tested-by: Song Liu 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 5fa0f5467d99..6bdd43d5dec5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -454,6 +454,7 @@ our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b}; our $logFunctions = qr{(?x: printk(?:_ratelimited|_once|_deferred_once|_deferred|)| (?:[a-z0-9]+_){1,2}(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)| + TP_printk| WARN(?:_RATELIMIT|_ONCE|)| panic| MODULE_[A-Z_]+| -- cgit v1.2.3 From 5751a24edfd43a91e072d63cde2b99b5a421645f Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 17 Nov 2017 15:28:52 -0800 Subject: checkpatch: add --strict test for lines ending in [ or ( Lines that end in an open bracket or open parenthesis are generally hard to follow. Lines following those ending with open parenthesis are also rarely aligned to that open parenthesis. Suggest not ending lines with '[' or '(' Link: http://lkml.kernel.org/r/8fd0b2b4a7482064254e37931eb9302a81d5aa2f.1508340786.git.joe@perches.com Signed-off-by: Joe Perches Suggested-by: Vivien Didelot Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- 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 6bdd43d5dec5..3453df9f90ab 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3184,6 +3184,12 @@ sub process { # check we are in a valid C source file if not then ignore this hunk next if ($realfile !~ /\.(h|c)$/); +# check for unusual line ending [ or ( + if ($line =~ /^\+.*([\[\(])\s*$/) { + CHK("OPEN_ENDED_LINE", + "Lines should not end with a '$1'\n" . $herecurr); + } + # check if this appears to be the start function declaration, save the name if ($sline =~ /^\+\{\s*$/ && $prevline =~ /^\+(?:(?:(?:$Storage|$Inline)\s*)*\s*$Type\s*)?($Ident)\(/) { -- cgit v1.2.3 From 0bc989ffc802752c5256192b4a9c8f16a00feca7 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 17 Nov 2017 15:28:55 -0800 Subject: checkpatch: do not check missing blank line before builtin_*_driver checkpatch.pl does not check missing blank line before module_*_driver. I want it to behave likewise for builtin_*_driver. Link: http://lkml.kernel.org/r/1505700081-12854-1-git-send-email-yamada.masahiro@socionext.com Signed-off-by: Masahiro Yamada Acked-by: Joe Perches 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 3453df9f90ab..95cda3ecc66b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3105,6 +3105,7 @@ sub process { $line =~ /^\+[a-z_]*init/ || $line =~ /^\+\s*(?:static\s+)?[A-Z_]*ATTR/ || $line =~ /^\+\s*DECLARE/ || + $line =~ /^\+\s*builtin_[\w_]*driver/ || $line =~ /^\+\s*__setup/)) { if (CHK("LINE_SPACING", "Please use a blank line after function/struct/union/enum declarations\n" . $hereprev) && -- cgit v1.2.3 From 7b1924a1d930eb27fc79c4e4e2a6c1c970623e68 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 23 Nov 2017 10:59:45 +1100 Subject: vsprintf: add printk specifier %px printk specifier %p now hashes all addresses before printing. Sometimes we need to see the actual unmodified address. This can be achieved using %lx but then we face the risk that if in future we want to change the way the Kernel handles printing of pointers we will have to grep through the already existent 50 000 %lx call sites. Let's add specifier %px as a clear, opt-in, way to print a pointer and maintain some level of isolation from all the other hex integer output within the Kernel. Add printk specifier %px to print the actual unmodified address. Signed-off-by: Tobin C. Harding --- Documentation/printk-formats.txt | 18 +++++++++++++++++- lib/vsprintf.c | 18 ++++++++++++++++++ scripts/checkpatch.pl | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt index b4e668ac4fe3..aa0a776c817a 100644 --- a/Documentation/printk-formats.txt +++ b/Documentation/printk-formats.txt @@ -49,7 +49,8 @@ Pointer Types Pointers printed without a specifier extension (i.e unadorned %p) are hashed to give a unique identifier without leaking kernel addresses to user -space. On 64 bit machines the first 32 bits are zeroed. +space. On 64 bit machines the first 32 bits are zeroed. If you _really_ +want the address see %px below. :: @@ -106,6 +107,21 @@ For printing kernel pointers which should be hidden from unprivileged users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see Documentation/sysctl/kernel.txt for more details. +Unmodified Addresses +==================== + +:: + + %px 01234567 or 0123456789abcdef + +For printing pointers when you _really_ want to print the address. Please +consider whether or not you are leaking sensitive information about the +Kernel layout in memory before printing pointers with %px. %px is +functionally equivalent to %lx. %px is preferred to %lx because it is more +uniquely grep'able. If, in the future, we need to modify the way the Kernel +handles printing pointers it will be nice to be able to find the call +sites. + Struct Resources ================ diff --git a/lib/vsprintf.c b/lib/vsprintf.c index d69452a0f2fa..d960aead0336 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1646,6 +1646,20 @@ char *device_node_string(char *buf, char *end, struct device_node *dn, return widen_string(buf, buf - buf_start, end, spec); } +static noinline_for_stack +char *pointer_string(char *buf, char *end, const void *ptr, + struct printf_spec spec) +{ + spec.base = 16; + spec.flags |= SMALL; + if (spec.field_width == -1) { + spec.field_width = 2 * sizeof(ptr); + spec.flags |= ZEROPAD; + } + + return number(buf, end, (unsigned long int)ptr, spec); +} + static bool have_filled_random_ptr_key __read_mostly; static siphash_key_t ptr_key __read_mostly; @@ -1818,6 +1832,8 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec) * c major compatible string * C full compatible string * + * - 'x' For printing the address. Equivalent to "%lx". + * * ** Please update also Documentation/printk-formats.txt when making changes ** * * Note: The difference between 'S' and 'F' is that on ia64 and ppc64 @@ -1940,6 +1956,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'F': return device_node_string(buf, end, ptr, spec, fmt + 1); } + case 'x': + return pointer_string(buf, end, ptr, spec); } /* default is to _not_ leak addresses, hash before printing */ diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 95cda3ecc66b..040aa79e1d9d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5753,7 +5753,7 @@ sub process { for (my $count = $linenr; $count <= $lc; $count++) { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { + if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNOx]).)/) { $bad_extension = $1; last; } -- cgit v1.2.3 From 91db2592e463157d8f4755f56230fb04d0308c4e Mon Sep 17 00:00:00 2001 From: "Paul E. McKenney" Date: Mon, 27 Nov 2017 09:37:35 -0800 Subject: checkpatch: Add warnings for {smp_,}read_barrier_depends() Now that both smp_read_barrier_depends() and read_barrier_depends() are being de-emphasized, warn if any are added. Signed-off-by: Paul E. McKenney Cc: Andy Whitcroft Cc: Joe Perches [ paulmck: Skipped checking files and handled whitespace per 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 95cda3ecc66b..9a384bfe2bd5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5586,6 +5586,12 @@ sub process { } } +# check for smp_read_barrier_depends and read_barrier_depends + if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) { + WARN("READ_BARRIER_DEPENDS", + "$1read_barrier_depends should only be used in READ_ONCE or DEC Alpha code\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", -- cgit v1.2.3 From 8cb562b1d56fad42cbee44bdc9bc64cea41a0a8c Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Mon, 27 Nov 2017 10:38:24 +0000 Subject: checkpatch: Remove ACCESS_ONCE() warning Now that ACCESS_ONCE() has been excised from the kernel, any uses will result in a build error, and we no longer need to whine about it in checkpatch. This patch removes the newly redundant warning. Tested-by: Paul E. McKenney Signed-off-by: Mark Rutland Acked-by: Joe Perches Cc: Andy Whitcroft Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: acme@redhat.com Link: http://lkml.kernel.org/r/20171127103824.36526-5-mark.rutland@arm.com Signed-off-by: Ingo Molnar --- scripts/checkpatch.pl | 22 ---------------------- 1 file changed, 22 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 040aa79e1d9d..31031f10fe56 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6233,28 +6233,6 @@ sub process { } } -# whine about ACCESS_ONCE - if ($^V && $^V ge 5.10.0 && - $line =~ /\bACCESS_ONCE\s*$balanced_parens\s*(=(?!=))?\s*($FuncArg)?/) { - my $par = $1; - my $eq = $2; - my $fun = $3; - $par =~ s/^\(\s*(.*)\s*\)$/$1/; - if (defined($eq)) { - if (WARN("PREFER_WRITE_ONCE", - "Prefer WRITE_ONCE(, ) over ACCESS_ONCE() = \n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)\s*$eq\s*\Q$fun\E/WRITE_ONCE($par, $fun)/; - } - } else { - if (WARN("PREFER_READ_ONCE", - "Prefer READ_ONCE() over ACCESS_ONCE()\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)/READ_ONCE($par)/; - } - } - } - # check for mutex_trylock_recursive usage if ($line =~ /mutex_trylock_recursive/) { ERROR("LOCKING", -- cgit v1.2.3 From 1df7338ac96558d5ae4c1a9dd5d1cb60fcd1bdb2 Mon Sep 17 00:00:00 2001 From: Sergey Senozhatsky Date: Fri, 10 Nov 2017 08:48:30 +0900 Subject: checkpatch: add pF/pf deprecation warning We deprecated '%pF/%pf' printk specifiers, since '%pS/%ps' is now smart enough to handle function pointer dereference on platforms where such dereference is required. Link: http://lkml.kernel.org/r/20171109234830.5067-7-sergey.senozhatsky@gmail.com To: Tony Luck To: Fenghua Yu To: Helge Deller To: Benjamin Herrenschmidt To: Paul Mackerras To: Michael Ellerman To: James Bottomley Cc: Andrew Morton Cc: Jessica Yu Cc: Petr Mladek Cc: Steven Rostedt Cc: linux-ia64@vger.kernel.org Cc: linux-parisc@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org Cc: Sergey Senozhatsky Cc: Sergey Senozhatsky Cc: Joe Perches Cc: Andy Whitcroft Signed-off-by: Sergey Senozhatsky Signed-off-by: Joe Perches Signed-off-by: Petr Mladek --- scripts/checkpatch.pl | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 95cda3ecc66b..0bb68e7ff173 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5753,18 +5753,25 @@ sub process { for (my $count = $linenr; $count <= $lc; $count++) { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { + if ($fmt =~ /(\%[\*\d\.]*p(?![\WSsBKRraEhMmIiUDdgVCbGNO]).)/) { $bad_extension = $1; last; } } if ($bad_extension ne "") { my $stat_real = raw_line($linenr, 0); + my $ext_type = "Invalid"; + my $use = ""; for (my $count = $linenr + 1; $count <= $lc; $count++) { $stat_real = $stat_real . "\n" . raw_line($count, 0); } + if ($bad_extension =~ /p[Ff]/) { + $ext_type = "Deprecated"; + $use = " - use %pS instead"; + $use =~ s/pS/ps/ if ($bad_extension =~ /pf/); + } WARN("VSPRINTF_POINTER_EXTENSION", - "Invalid vsprintf pointer extension '$bad_extension'\n" . "$here\n$stat_real\n"); + "$ext_type vsprintf pointer extension '$bad_extension'$use\n" . "$here\n$stat_real\n"); } } -- cgit v1.2.3 From 2e4bbbc550be336cbb3defc67430fc0700aa1426 Mon Sep 17 00:00:00 2001 From: Andreas Brauchli Date: Tue, 6 Feb 2018 15:38:45 -0800 Subject: checkpatch: allow long lines containing URL Allow lines with URL to exceed the 80 char limit for improved interaction in adaption to ongoing but undocumented practice. $ git grep -E '://\S{77}.*' -- '*.[ch]' As per RFC3986 [1], the URL format allows for alphanum, +, - and . characters in the scheme before the separator :// as long as it starts with a letter (e.g. https, git, f.-+). Recognition of URIs without more context information is prone to false positives and thus currently left out of the heuristics. $rawline is used in the check as comments are removed from $line. [1] https://tools.ietf.org/html/rfc3986#section-3.1 Link: http://lkml.kernel.org/r/1511355432.12667.15.camel@elementarea.net Signed-off-by: Andreas Brauchli Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e954df2b2077..4306b7616cdd 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2875,6 +2875,7 @@ sub process { # logging functions like pr_info that end in a string # lines with a single string # #defines that are a single string +# lines with an RFC3986 like URL # # There are 3 different line length message types: # LONG_LINE_COMMENT a comment starts before but extends beyond $max_line_length @@ -2906,6 +2907,10 @@ sub process { $line =~ /^\+\s*(?:\w+)?\s*DEFINE_PER_CPU/) { $msg_type = ""; + # URL ($rawline is used in case the URL is in a comment) + } elsif ($rawline =~ /^\+.*\b[a-z][\w\.\+\-]*:\/\/\S+/i) { + $msg_type = ""; + # Otherwise set the alternate message types # a comment starts before $max_line_length -- cgit v1.2.3 From 73121534c963f7b6d22547a9ad8bff1793748433 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 6 Feb 2018 15:38:49 -0800 Subject: checkpatch: ignore some octal permissions of 0 module_param and create_proc uses with a permissions use of a single 0 are "special" and should not emit any warning. module_param uses with permission 0 are not visible in sysfs create_proc uses with permission 0 use a default permission Link: http://lkml.kernel.org/r/b6583611bb529ea6f6d43786827fddbabbab0a71.1513190059.git.joe@perches.com Signed-off-by: Bartosz Golaszewski Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4306b7616cdd..168687ae24fa 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6276,6 +6276,10 @@ 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 +# o Ignore module_param*(...) uses with a decimal 0 permission as that has a +# specific definition of not visible in sysfs. +# o Ignore proc_create*(...) uses with a decimal 0 permission as that means +# use the default permissions if ($^V && $^V ge 5.10.0 && defined $stat && $line =~ /$mode_perms_search/) { @@ -6299,8 +6303,9 @@ sub process { if ($stat =~ /$test/) { my $val = $1; $val = $6 if ($skip_args ne ""); - if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || - ($val =~ /^$Octal$/ && length($val) ne 4)) { + if (!($func =~ /^(?:module_param|proc_create)/ && $val eq "0") && + (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || + ($val =~ /^$Octal$/ && length($val) ne 4))) { ERROR("NON_OCTAL_PERMISSIONS", "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real); } -- cgit v1.2.3 From 3f7f335dbc8675e0b55b2cedb5164b7ead1e4fe6 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 6 Feb 2018 15:38:52 -0800 Subject: checkpatch: improve quoted string and line continuation test Given this patch context, +#define EFI_ST_DISK_IMG { \ + 0x00000240, "\xbe\x5b\x7c\xac\x22\xc0\x74\x0b" /* .[|.".t. */ \ + } the current code misreports a quoted string line continuation defect as there is a single quote in comment. The 'raw' line should not be tested for quote count, the comment substituted line should be instead. Link: http://lkml.kernel.org/r/13f2735df10c33ca846e26f42f5cce6618157200.1513698599.git.joe@perches.com Signed-off-by: Joe Perches Reported-by: Heinrich Schuchardt 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 168687ae24fa..7c635146cb80 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5312,7 +5312,7 @@ sub process { } # check for line continuations in quoted strings with odd counts of " - if ($rawline =~ /\\$/ && $rawline =~ tr/"/"/ % 2) { + if ($rawline =~ /\\$/ && $sline =~ tr/"/"/ % 2) { WARN("LINE_CONTINUATIONS", "Avoid line continuations in quoted strings\n" . $herecurr); } -- cgit v1.2.3 From 001804689b0d41410ae483d3e3dcdb16c3a6640f Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 6 Feb 2018 15:38:55 -0800 Subject: checkpatch: add a few DEVICE_ATTR style tests DEVICE_ATTR is a declaration macro that has a few alternate and preferred forms like DEVICE_ATTR_RW, DEVICE_ATTR_RO, and DEVICE_ATTR. As well, many uses of DEVICE_ATTR could use the preferred forms when the show or store functions are also named in a regular form. Suggest the preferred forms when appropriate. Also emit a permissions warning if the the permissions are not the typical 0644, 0444, or 0200. Link: http://lkml.kernel.org/r/725864f363d91d1e1e6894a39fb57662eabd6d65.1513803306.git.joe@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 114 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 93 insertions(+), 21 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7c635146cb80..51905634be23 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -566,6 +566,7 @@ foreach my $entry (@mode_permission_funcs) { $mode_perms_search .= '|' if ($mode_perms_search ne ""); $mode_perms_search .= $entry->[0]; } +$mode_perms_search = "(?:${mode_perms_search})"; our $mode_perms_world_writable = qr{ S_IWUGO | @@ -600,6 +601,37 @@ foreach my $entry (keys %mode_permission_string_types) { $mode_perms_string_search .= '|' if ($mode_perms_string_search ne ""); $mode_perms_string_search .= $entry; } +our $single_mode_perms_string_search = "(?:${mode_perms_string_search})"; +our $multi_mode_perms_string_search = qr{ + ${single_mode_perms_string_search} + (?:\s*\|\s*${single_mode_perms_string_search})* +}x; + +sub perms_to_octal { + my ($string) = @_; + + return trim($string) if ($string =~ /^\s*0[0-7]{3,3}\s*$/); + + my $val = ""; + my $oval = ""; + my $to = 0; + my $curpos = 0; + my $lastpos = 0; + while ($string =~ /\b(($single_mode_perms_string_search)\b(?:\s*\|\s*)?\s*)/g) { + $curpos = pos($string); + 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*$//; + return sprintf("%04o", $to); +} our $allowed_asm_includes = qr{(?x: irq| @@ -6274,6 +6306,63 @@ sub process { "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } +# check for DEVICE_ATTR uses that could be DEVICE_ATTR_ +# and whether or not function naming is typical and if +# DEVICE_ATTR permissions uses are unusual too + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?\s*(\s*(?:${multi_mode_perms_string_search}|0[0-7]{3,3})\s*)\s*\)?\s*,\s*(\w+)\s*,\s*(\w+)\s*\)/) { + my $var = $1; + my $perms = $2; + my $show = $3; + my $store = $4; + my $octal_perms = perms_to_octal($perms); + if ($show =~ /^${var}_show$/ && + $store =~ /^${var}_store$/ && + $octal_perms eq "0644") { + if (WARN("DEVICE_ATTR_RW", + "Use DEVICE_ATTR_RW\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bDEVICE_ATTR\s*\(\s*$var\s*,\s*\Q$perms\E\s*,\s*$show\s*,\s*$store\s*\)/DEVICE_ATTR_RW(${var})/; + } + } elsif ($show =~ /^${var}_show$/ && + $store =~ /^NULL$/ && + $octal_perms eq "0444") { + if (WARN("DEVICE_ATTR_RO", + "Use DEVICE_ATTR_RO\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bDEVICE_ATTR\s*\(\s*$var\s*,\s*\Q$perms\E\s*,\s*$show\s*,\s*NULL\s*\)/DEVICE_ATTR_RO(${var})/; + } + } elsif ($show =~ /^NULL$/ && + $store =~ /^${var}_store$/ && + $octal_perms eq "0200") { + if (WARN("DEVICE_ATTR_WO", + "Use DEVICE_ATTR_WO\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bDEVICE_ATTR\s*\(\s*$var\s*,\s*\Q$perms\E\s*,\s*NULL\s*,\s*$store\s*\)/DEVICE_ATTR_WO(${var})/; + } + } elsif ($octal_perms eq "0644" || + $octal_perms eq "0444" || + $octal_perms eq "0200") { + my $newshow = "$show"; + $newshow = "${var}_show" if ($show ne "NULL" && $show ne "${var}_show"); + my $newstore = $store; + $newstore = "${var}_store" if ($store ne "NULL" && $store ne "${var}_store"); + my $rename = ""; + if ($show ne $newshow) { + $rename .= " '$show' to '$newshow'"; + } + if ($store ne $newstore) { + $rename .= " '$store' to '$newstore'"; + } + WARN("DEVICE_ATTR_FUNCTIONS", + "Consider renaming function(s)$rename\n" . $herecurr); + } else { + WARN("DEVICE_ATTR_PERMS", + "DEVICE_ATTR unusual permissions '$perms' used\n" . $herecurr); + } + } + # Mode permission misuses where it seems decimal should be octal # This uses a shortcut match to avoid unnecessary uses of a slow foreach loop # o Ignore module_param*(...) uses with a decimal 0 permission as that has a @@ -6318,30 +6407,13 @@ sub process { } # 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 ($line =~ /\b($multi_mode_perms_string_search)\b/) { + my $oval = $1; + my $octal = perms_to_octal($oval); if (WARN("SYMBOLIC_PERMS", "Symbolic permissions '$oval' are not preferred. Consider using octal permissions '$octal'.\n" . $herecurr) && $fix) { - $fixed[$fixlinenr] =~ s/$val/$octal/; + $fixed[$fixlinenr] =~ s/\Q$oval\E/$octal/; } } -- cgit v1.2.3 From a134f8de9f4014dad35884b3cc211028f0a76c7a Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 6 Feb 2018 15:38:59 -0800 Subject: checkpatch: improve the TABSTOP test to include declarations Declarations should start on a tabstop too. Link: http://lkml.kernel.org/r/1b5f97673f36595956ad43329f77bf1a5546d2ff.1513976662.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 51905634be23..d2464058ab5d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3020,7 +3020,7 @@ sub process { # check indentation starts on a tab stop if ($^V && $^V ge 5.10.0 && - $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$))/) { + $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=\[])/) { my $indent = length($1); if ($indent % 8) { if (WARN("TABSTOP", -- cgit v1.2.3 From a032aa4c4a54c25f204358248db08a0966d3513c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 6 Feb 2018 15:39:03 -0800 Subject: checkpatch: exclude drivers/staging from if with unnecessary parentheses test Greg KH doesn't like this test so exclude the staging directory from the implied --strict only test unless --strict is actually used on the command-line. Link: http://lkml.kernel.org/r/1515704034.9619.165.camel@perches.com Signed-off-by: Joe Perches Cc: Greg KH Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d2464058ab5d..3a7499de2c2d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4526,7 +4526,9 @@ sub process { } # check for unnecessary parentheses around comparisons in if uses - if ($^V && $^V ge 5.10.0 && defined($stat) && +# when !drivers/staging or command-line uses --strict + if (($realfile !~ m@^(?:drivers/staging/)@ || $check_orig) && + $^V && $^V ge 5.10.0 && defined($stat) && $stat =~ /(^.\s*if\s*($balanced_parens))/) { my $if_stat = $1; my $test = substr($2, 1, -1); -- cgit v1.2.3 From bd49111f7dd19a645ce85fb24a0ea52d82ca2e3c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 6 Feb 2018 15:39:06 -0800 Subject: checkpatch: avoid some false positives for TABSTOP declaration test Using an open bracket after what seems to be a declaration can also be a function definition and declaration argument line continuation so remove the open bracket from the possible declaration/definition matching. e.g.: int foobar(int a; int *b[]); Link: http://lkml.kernel.org/r/1515704479.9619.171.camel@perches.com Signed-off-by: Joe Perches Reported-by: Sven Eckelmann 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 3a7499de2c2d..78e7a310af46 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3020,7 +3020,7 @@ sub process { # check indentation starts on a tab stop if ($^V && $^V ge 5.10.0 && - $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=\[])/) { + $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=])/) { my $indent = length($1); if ($indent % 8) { if (WARN("TABSTOP", -- cgit v1.2.3 From 2d453e3b41c80d1a2c02b02d672f5dcd73f95a12 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 6 Feb 2018 15:39:09 -0800 Subject: checkpatch: improve OPEN_BRACE test Some structure definitions that use macros trip the OPEN_BRACE test. e.g. +struct bpf_map_def SEC("maps") control_map = { Improve the test by using $balanced_parens instead of a .* Miscellanea: o Use $sline so any comments are ignored o Correct the message output from declaration to definition o Remove unnecessary parentheses Link: http://lkml.kernel.org/r/db9b772999d1d2fbda3b9ee24bbca81a87837e13.1517543491.git.joe@perches.com Signed-off-by: Joe Perches Reported-by: Song Liu Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 78e7a310af46..3d4040322ae1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3919,10 +3919,12 @@ sub process { # function brace can't be on same line, except for #defines of do while, # or if closed on same line - if (($line=~/$Type\s*$Ident\(.*\).*\s*{/) and - !($line=~/\#\s*define.*do\s\{/) and !($line=~/}/)) { + if ($^V && $^V ge 5.10.0 && + $sline =~ /$Type\s*$Ident\s*$balanced_parens\s*\{/ && + $sline !~ /\#\s*define\b.*do\s*\{/ && + $sline !~ /}/) { if (ERROR("OPEN_BRACE", - "open brace '{' following function declarations go on the next line\n" . $herecurr) && + "open brace '{' following function definitions go on the next line\n" . $herecurr) && $fix) { fix_delete_line($fixlinenr, $rawline); my $fixed_line = $rawline; -- cgit v1.2.3