checkpatch: fix false EXPORT_SYMBOL warning
authorAndy Whitcroft <apw@canonical.com>
Mon, 26 Oct 2009 23:50:16 +0000 (16:50 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 29 Oct 2009 14:39:31 +0000 (07:39 -0700)
Ingo reported that the following lines triggered a false warning,

static struct lock_class_key rcu_lock_key;
struct lockdep_map rcu_lock_map =
        STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", &rcu_lock_key);
EXPORT_SYMBOL_GPL(rcu_lock_map);

from kernel/rcutree.c , and the false warning looked like this,

WARNING: EXPORT_SYMBOL(foo); should immediately follow its
function/variable
+EXPORT_SYMBOL_GPL(rcu_lock_map);

We actually should be checking the statement before the EXPORT_* for a
mention of the exported object, and complain where it is not there.

[akpm@linux-foundation.org: coding-style fixes]
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reported-by: Daniel Walker <dwalker@fifo99.com>
Signed-off-by: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
scripts/checkpatch.pl

index ba105a84839674acfa069baab2508c926cadd5ad..88c4f6a5080b55a5ec17937bac6a00866101927a 100755 (executable)
@@ -1145,6 +1145,7 @@ sub process {
        # suppression flags
        my %suppress_ifbraces;
        my %suppress_whiletrailers;
+       my %suppress_export;
 
        # Pre-scan the patch sanitizing the lines.
        # Pre-scan the patch looking for any __setup documentation.
@@ -1253,6 +1254,7 @@ sub process {
 
                        %suppress_ifbraces = ();
                        %suppress_whiletrailers = ();
+                       %suppress_export = ();
                        next;
 
 # track the line number as we move through the hunk, note that
@@ -1428,13 +1430,22 @@ sub process {
                }
 
 # Check for potential 'bare' types
-               my ($stat, $cond, $line_nr_next, $remain_next, $off_next);
+               my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
+                   $realline_next);
                if ($realcnt && $line =~ /.\s*\S/) {
                        ($stat, $cond, $line_nr_next, $remain_next, $off_next) =
                                ctx_statement_block($linenr, $realcnt, 0);
                        $stat =~ s/\n./\n /g;
                        $cond =~ s/\n./\n /g;
 
+                       # Find the real next line.
+                       $realline_next = $line_nr_next;
+                       if (defined $realline_next &&
+                           (!defined $lines[$realline_next - 1] ||
+                            substr($lines[$realline_next - 1], $off_next) =~ /^\s*$/)) {
+                               $realline_next++;
+                       }
+
                        my $s = $stat;
                        $s =~ s/{.*$//s;
 
@@ -1695,21 +1706,40 @@ sub process {
                $line =~ s@//.*@@;
                $opline =~ s@//.*@@;
 
-#EXPORT_SYMBOL should immediately follow its function closing }.
-               if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) ||
-                   ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+# EXPORT_SYMBOL should immediately follow the thing it is exporting, consider
+# the whole statement.
+#print "APW <$lines[$realline_next - 1]>\n";
+               if (defined $realline_next &&
+                   exists $lines[$realline_next - 1] &&
+                   !defined $suppress_export{$realline_next} &&
+                   ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
+                    $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
                        my $name = $1;
-                       if ($prevline !~ /(?:
-                               ^.}|
+                       if ($stat !~ /(?:
+                               \n.}\s*$|
                                ^.DEFINE_$Ident\(\Q$name\E\)|
                                ^.DECLARE_$Ident\(\Q$name\E\)|
                                ^.LIST_HEAD\(\Q$name\E\)|
-                               ^.$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
-                               \b\Q$name\E(?:\s+$Attribute)?\s*(?:;|=|\[)
+                               ^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
+                               \b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
                            )/x) {
-                               WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr);
+#print "FOO A<$lines[$realline_next - 1]> stat<$stat> name<$name>\n";
+                               $suppress_export{$realline_next} = 2;
+                       } else {
+                               $suppress_export{$realline_next} = 1;
                        }
                }
+               if (!defined $suppress_export{$linenr} &&
+                   $prevline =~ /^.\s*$/ &&
+                   ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
+                    $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+#print "FOO B <$lines[$linenr - 1]>\n";
+                       $suppress_export{$linenr} = 2;
+               }
+               if (defined $suppress_export{$linenr} &&
+                   $suppress_export{$linenr} == 2) {
+                       WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr);
+               }
 
 # check for external initialisers.
                if ($line =~ /^.$Type\s*$Ident\s*(?:\s+$Modifier)*\s*=\s*(0|NULL|false)\s*;/) {