From 8f53a9b80f011080555c498d2ca2dc6b1a77c42c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 5 Mar 2010 13:43:48 -0800 Subject: scripts/checkpatch.pl: add WARN on sizeof(&foo) sizeof(&foo) is frequently an error. Warn on its use. Signed-off-by: Joe Perches Cc: Andy Whitcroft 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 3257d3d96767..309050f30874 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2572,6 +2572,11 @@ sub process { WARN("plain inline is preferred over $1\n" . $herecurr); } +# check for sizeof(&) + if ($line =~ /\bsizeof\s*\(\s*\&/) { + WARN("sizeof(& should be avoided\n" . $herecurr); + } + # check for new externs in .c files. if ($realfile =~ /\.c$/ && defined $stat && $stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s) -- cgit v1.2.3 From 42bdf74c95b6935f3c09a09ba4bead6cad11b540 Mon Sep 17 00:00:00 2001 From: Hidetoshi Seto Date: Fri, 5 Mar 2010 13:43:50 -0800 Subject: checkpatch: trivial fix for trailing statements check In case if the statement and the conditional are in one line, the line appears in the report doubly. And items of this check have no blank line before the next item. This patch fixes these trivial problems, to improve readability of the report. [sample.c] > if (cond1 > && cond2 > && cond3) func_foo(); > > if (cond4) func_bar(); Before: > ERROR: trailing statements should be on next line > #1: FILE: sample.c:1: > +if (cond1 > [...] > + && cond3) func_foo(); > ERROR: trailing statements should be on next line > #5: FILE: sample.c:5: > +if (cond4) func_bar(); > +if (cond4) func_bar(); > total: 2 errors, 0 warnings, 5 lines checked After: > ERROR: trailing statements should be on next line > #1: FILE: sample.c:1: > +if (cond1 > [...] > + && cond3) func_foo(); > > ERROR: trailing statements should be on next line > #5: FILE: sample.c:5: > +if (cond4) func_bar(); > > total: 2 errors, 0 warnings, 5 lines checked Signed-off-by: Hidetoshi Seto Cc: Andy Whitcroft 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 309050f30874..3e4099a6df51 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2182,8 +2182,10 @@ sub process { # Find out how long the conditional actually is. my @newlines = ($c =~ /\n/gs); my $cond_lines = 1 + $#newlines; + my $stat_real = ''; - my $stat_real = raw_line($linenr, $cond_lines); + $stat_real = raw_line($linenr, $cond_lines) + . "\n" if ($cond_lines); if (defined($stat_real) && $cond_lines > 1) { $stat_real = "[...]\n$stat_real"; } -- cgit v1.2.3 From 691e669ba8c64d31ac08d87b1751e6acfa3ff65e Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 5 Mar 2010 13:43:51 -0800 Subject: checkpatch.pl: allow > 80 char lines for logging functions not just printk Signed-off-by: Joe Perches Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3e4099a6df51..9a3894dbf752 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -189,6 +189,14 @@ our $typeTypedefs = qr{(?x: atomic_t )}; +our $logFunctions = qr{(?x: + printk| + pr_(debug|dbg|vdbg|devel|info|warning|err|notice|alert|crit|emerg|cont)| + dev_(printk|dbg|vdbg|info|warn|err|notice|alert|crit|emerg|WARN)| + WARN| + panic +)}; + our @typeList = ( qr{void}, qr{(?:unsigned\s+)?char}, @@ -1377,7 +1385,7 @@ sub process { #80 column limit if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && $rawline !~ /^.\s*\*\s*\@$Ident\s/ && - $line !~ /^\+\s*printk\s*\(\s*(?:KERN_\S+\s*)?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ && + $line !~ /^\+\s*$logFunctions\s*\(\s*(?:KERN_\S+\s*)?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ && $length > 80) { WARN("line over 80 characters\n" . $herecurr); -- cgit v1.2.3 From 52131292c069b74f4b5f3c786ff66ff6e82b0e69 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Fri, 5 Mar 2010 13:43:51 -0800 Subject: checkpatch: fix false positive on __initconst checkpatch falsely complained about '__initconst' because it thought the 'const' needed a space before. Fix this by changing the list of attributes: - add '__initconst' - force plain 'init' to contain a word-boundary at the end Signed-off-by: Wolfram Sang Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9a3894dbf752..1530dbf1dab5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -145,11 +145,14 @@ our $Sparse = qr{ __kprobes| __ref }x; + +# Notes to $Attribute: +# We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| __read_mostly| __kprobes| - __(?:mem|cpu|dev|)(?:initdata|init)| + __(?:mem|cpu|dev|)(?:initdata|initconst|init\b)| ____cacheline_aligned| ____cacheline_aligned_in_smp| ____cacheline_internodealigned_in_smp| -- cgit v1.2.3 From 22fd2d3e4f75a2596ccdfdcbdfd505c9c60bf346 Mon Sep 17 00:00:00 2001 From: Stefani Seibold Date: Fri, 5 Mar 2010 13:43:52 -0800 Subject: checkpatch.pl: add union and struct to the exceptions list Here is a small code snippet, which will be complained about by checkpatch.pl: #define __STRUCT_KFIFO_COMMON(recsize, ptrtype) \ union { \ struct { \ unsigned int in; \ unsigned int out; \ }; \ char rectype[recsize]; \ ptrtype *ptr; \ const ptrtype *ptr_const; \ }; This construct is legal and safe, so checkpatch.pl should accept this. It should be also true for struct defined in a macro. Add the `struct' and `union' keywords to the exceptions list of the checkpatch.pl script, to prevent error message "Macros with multiple statements should be enclosed in a do - while loop". Otherwise it is not possible to build a struct or union with a macro. Signed-off-by: Stefani Seibold Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1530dbf1dab5..623d07b97727 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2361,6 +2361,8 @@ sub process { DECLARE_PER_CPU| DEFINE_PER_CPU| __typeof__\(| + union| + struct| \.$Ident\s*=\s*| ^\"|\"$ }x; -- cgit v1.2.3 From 79404849e90a41ea2109bd0e2f7c7164b0c4ce73 Mon Sep 17 00:00:00 2001 From: Emese Revfy Date: Fri, 5 Mar 2010 13:43:53 -0800 Subject: checkpatch.pl: extend list of expected-to-be-const structures Based on Arjan's suggestion, extend the list of ops structures that should be const. Signed-off-by: Emese Revfy Cc: Andy Whitcroft Cc: Arjan van de Ven Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 623d07b97727..1a93ac265c31 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2654,9 +2654,46 @@ sub process { if ($line =~ /^.\s*__initcall\s*\(/) { WARN("please use device_initcall() instead of __initcall()\n" . $herecurr); } -# check for struct file_operations, ensure they are const. +# check for various ops structs, ensure they are const. + my $struct_ops = 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| + 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| + usb_mon_operations| + wd_ops}x; if ($line !~ /\bconst\b/ && - $line =~ /\bstruct\s+(file_operations|seq_operations)\b/) { + $line =~ /\bstruct\s+($struct_ops)\b/) { WARN("struct $1 should normally be const\n" . $herecurr); } -- cgit v1.2.3 From 08e4436566250cb98b3f3ac37643b1cf09481256 Mon Sep 17 00:00:00 2001 From: Alberto Panizzo Date: Fri, 5 Mar 2010 13:43:54 -0800 Subject: checkpatch.pl: warn if an adding line introduce spaces before tabs. Signed-off-by: Alberto Panizzo Cc: Andy Whitcroft 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 1a93ac265c31..cf4e44cf3c27 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1422,6 +1422,12 @@ sub process { ERROR("code indent should use tabs where possible\n" . $herevet); } +# check for space before tabs. + if ($rawline =~ /^\+/ && $rawline =~ / \t/) { + my $herevet = "$here\n" . cat_vet($rawline) . "\n"; + WARN("please, no space before tabs\n" . $herevet); + } + # check we are in a valid C source file if not then ignore this hunk next if ($realfile !~ /\.(h|c)$/); -- cgit v1.2.3 From 5e79d96eed306a8b4af67b3f35f6867edfabeebc Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 5 Mar 2010 13:43:55 -0800 Subject: checkpatch: warn on unnecessary spaces before quoted newlines Signed-off-by: Joe Perches Cc: Andy Whitcroft 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 cf4e44cf3c27..a4d74344d805 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1394,6 +1394,11 @@ sub process { WARN("line over 80 characters\n" . $herecurr); } +# check for spaces before a quoted newline + if ($rawline =~ /^.*\".*\s\\n/) { + WARN("unnecessary whitespace before a quoted newline\n" . $herecurr); + } + # check for adding lines without a newline. if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) { WARN("adding a line without newline at end of file\n" . $herecurr); -- cgit v1.2.3 From 1704f47b50b5d9e1b825e43e1baaf2c5897baf03 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Fri, 19 Mar 2010 01:37:42 +0100 Subject: lockdep: Add novalidate class for dev->mutex conversion The conversion of device->sem to device->mutex resulted in lockdep warnings. Create a novalidate class for now until the driver folks come up with separate classes. That way we have at least the basic mutex debugging coverage. Add a checkpatch error so the usage is reserved for device->mutex. [ tglx: checkpatch and compile fix for LOCKDEP=n ] Signed-off-by: Peter Zijlstra Signed-off-by: Thomas Gleixner Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 1 + include/linux/lockdep.h | 8 ++++++++ kernel/lockdep.c | 5 +++++ scripts/checkpatch.pl | 11 +++++++++++ 4 files changed, 25 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/drivers/base/core.c b/drivers/base/core.c index cf507a7d200c..4c5be85016b6 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -559,6 +559,7 @@ void device_initialize(struct device *dev) kobject_init(&dev->kobj, &device_ktype); INIT_LIST_HEAD(&dev->dma_pools); mutex_init(&dev->mutex); + lockdep_set_novalidate_class(&dev->mutex); spin_lock_init(&dev->devres_lock); INIT_LIST_HEAD(&dev->devres_head); device_pm_init(dev); diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index a03977a96d7e..06aed8305bf3 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -44,6 +44,8 @@ struct lock_class_key { struct lockdep_subclass_key subkeys[MAX_LOCKDEP_SUBCLASSES]; }; +extern struct lock_class_key __lockdep_no_validate__; + #define LOCKSTAT_POINTS 4 /* @@ -270,6 +272,9 @@ extern void lockdep_init_map(struct lockdep_map *lock, const char *name, #define lockdep_set_subclass(lock, sub) \ lockdep_init_map(&(lock)->dep_map, #lock, \ (lock)->dep_map.key, sub) + +#define lockdep_set_novalidate_class(lock) \ + lockdep_set_class(lock, &__lockdep_no_validate__) /* * Compare locking classes */ @@ -354,6 +359,9 @@ static inline void lockdep_on(void) #define lockdep_set_class_and_subclass(lock, key, sub) \ do { (void)(key); } while (0) #define lockdep_set_subclass(lock, sub) do { } while (0) + +#define lockdep_set_novalidate_class(lock) do { } while (0) + /* * We don't define lockdep_match_class() and lockdep_match_key() for !LOCKDEP * case since the result is not well defined and the caller should rather diff --git a/kernel/lockdep.c b/kernel/lockdep.c index ec21304856d1..54286798c37b 100644 --- a/kernel/lockdep.c +++ b/kernel/lockdep.c @@ -2711,6 +2711,8 @@ void lockdep_init_map(struct lockdep_map *lock, const char *name, } EXPORT_SYMBOL_GPL(lockdep_init_map); +struct lock_class_key __lockdep_no_validate__; + /* * This gets called for every mutex_lock*()/spin_lock*() operation. * We maintain the dependency maps and validate the locking attempt: @@ -2745,6 +2747,9 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, return 0; } + if (lock->key == &__lockdep_no_validate__) + check = 1; + if (!subclass) class = lock->class_cache; /* diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index a4d74344d805..f2bbea900700 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2656,6 +2656,7 @@ sub process { # check for semaphores used as mutexes if ($line =~ /^.\s*init_MUTEX_LOCKED\s*\(/) { WARN("consider using a completion\n" . $herecurr); + } # recommend strict_strto* over simple_strto* if ($line =~ /\bsimple_(strto.*?)\s*\(/) { @@ -2740,6 +2741,16 @@ sub process { WARN("use of in_atomic() is incorrect outside core kernel code\n" . $herecurr); } } + +# check for lockdep_set_novalidate_class + if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ || + $line =~ /__lockdep_no_validate__\s*\)/ ) { + if ($realfile !~ m@^kernel/lockdep@ && + $realfile !~ m@^include/linux/lockdep@ && + $realfile !~ m@^drivers/base/core@) { + ERROR("lockdep_no_validate class is reserved for device->mutex.\n" . $herecurr); + } + } } # If we have no input at all, then there is nothing to report on -- cgit v1.2.3 From 3354957a4f8b9bb4b43625232acdf0626851c82f Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 24 May 2010 14:33:29 -0700 Subject: checkpatch: add check for too short Kconfig descriptions I've seen various new Kconfigs with rather unhelpful one liner descriptions. Add a Kconfig warning for a minimum length of the Kconfig help section. Right now I arbitarily chose 4. The exact value can be debated. [akpm@linux-foundation.org: coding-style fixes] Signed-off-by: Andi Kleen Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f2bbea900700..de0f4672cb74 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1382,6 +1382,21 @@ sub process { ERROR("trailing whitespace\n" . $herevet); } +# check for Kconfig help text having a real description + if ($realfile =~ /Kconfig/ && + $line =~ /\+?\s*(---)?help(---)?$/) { + my $length = 0; + for (my $l = $linenr; defined($lines[$l]); $l++) { + my $f = $lines[$l]; + $f =~ s/#.*//; + $f =~ s/^\s+//; + next if ($f =~ /^$/); + last if ($f =~ /^\s*config\s/); + $length++; + } + WARN("please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($length < 4); + } + # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); -- cgit v1.2.3 From d4977c78e9c7dd042f96f4a21d957bc25a561333 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Mon, 24 May 2010 14:33:30 -0700 Subject: checkpatch: warn on declaration with storage class not at the beginning The C99 specification states in section 6.11.5: The placement of a storage-class specifier other than at the beginning of the declaration specifiers in a declaration is an obsolescent feature. Signed-off-by: Tobias Klauser Acked-by: Jean Delvare Cc: Andy Whitcroft 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 de0f4672cb74..bd88f11b0953 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2601,6 +2601,11 @@ sub process { CHK("architecture specific defines should be avoided\n" . $herecurr); } +# Check that the storage class is at the beginning of a declaration + if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { + WARN("storage class should be at the beginning of the declaration\n" . $herecurr) + } + # check the location of the inline attribute, that it is between # storage class and type. if ($line =~ /\b$Type\s+$Inline\b/ || -- cgit v1.2.3