From 653d4876b730fedca8473481863cf700245e3582 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Sat, 23 Jun 2007 17:16:34 -0700 Subject: update checkpatch.pl to version 0.05 This version brings a some new tests, and a host of changes to fix false positives, of particular note: - detect 'var ++;' and 'var --;' as a bad combination - multistatement #defines are now checked based on statement count - multistatement #defines with initialisation correctly reported - checks the location of the inline keywords - EXPORT_SYMBOL for variables are now understood - typedefs are loosened to handle sparse etc This version of checkpatch.pl can be found at the following URL: http://www.shadowen.org/~apw/public/checkpatch/checkpatch.pl-0.05 Full Changelog: Andy Whitcroft (18): Version: 0.05 macro definition checks should be for a single statement avoid assignements only in if conditionals declarations of function pointers need no space multiline macros which are purely initialisation cannot be wrapped EXPORT_SYMBOL can also directly follow a variable definition check on the location of the inline keyword EXPORT_SYMBOL needs to allow for attributes ensure we do not find C99 // in strings handle malformed #include lines accept the {0,} form typedefs are sensible for defining function pointer parameters ensure { handling correctly handles nested switch() statements trailing whitespace checks are not anchored typedefs for sparse bitwise annotations make sense update the type matcher to include sparse annotations clean up indent and spacing Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 183 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 129 insertions(+), 54 deletions(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index aea90d30d229..56c364c1df81 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,7 +9,7 @@ use strict; my $P = $0; $P =~ s@.*/@@g; -my $V = '0.04'; +my $V = '0.05'; use Getopt::Long qw(:config no_auto_abbrev); @@ -17,11 +17,13 @@ my $quiet = 0; my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; +my $tst_type = 0; GetOptions( 'q|quiet' => \$quiet, 'tree!' => \$tree, 'signoff!' => \$chk_signoff, 'patch!' => \$chk_patch, + 'test-type!' => \$tst_type, ) or exit; my $exit = 0; @@ -151,7 +153,7 @@ sub sanitise_line { } sub ctx_block_get { - my ($linenr, $remain, $outer) = @_; + my ($linenr, $remain, $outer, $open, $close) = @_; my $line; my $start = $linenr - 1; my $blk = ''; @@ -165,8 +167,8 @@ sub ctx_block_get { $blk .= $rawlines[$line]; - @o = ($blk =~ /\{/g); - @c = ($blk =~ /\}/g); + @o = ($blk =~ /$open/g); + @c = ($blk =~ /$close/g); if (!$outer || (scalar(@o) - scalar(@c)) == 1) { push(@res, $rawlines[$line]); @@ -180,12 +182,17 @@ sub ctx_block_get { sub ctx_block_outer { my ($linenr, $remain) = @_; - return ctx_block_get($linenr, $remain, 1); + return ctx_block_get($linenr, $remain, 1, '\{', '\}'); } sub ctx_block { my ($linenr, $remain) = @_; - return ctx_block_get($linenr, $remain, 0); + return ctx_block_get($linenr, $remain, 0, '\{', '\}'); +} +sub ctx_statement { + my ($linenr, $remain) = @_; + + return ctx_block_get($linenr, $remain, 0, '\(', '\)'); } sub ctx_locate_comment { @@ -264,9 +271,30 @@ sub process { my $in_comment = 0; my $first_line = 0; + my $ident = '[A-Za-z\d_]+'; + my $storage = '(?:extern|static)'; + my $sparse = '(?:__user|__kernel|__force|__iomem)'; + my $type = '(?:unsigned\s+)?' . + '(?:void|char|short|int|long|unsigned|float|double|' . + 'long\s+long|' . + "struct\\s+${ident}|" . + "union\\s+${ident}|" . + "${ident}_t)" . + "(?:\\s+$sparse)*" . + '(?:\s*\*+)?'; + my $attribute = '(?:__read_mostly|__init|__initdata)'; + + my $Ident = $ident; + my $Type = $type; + my $Storage = $storage; + my $Declare = "(?:$storage\\s+)?$type"; + my $Attribute = $attribute; + foreach my $line (@lines) { $linenr++; + my $rawline = $line; + #extract the filename as it passes if ($line=~/^\+\+\+\s+(\S+)/) { $realfile=$1; @@ -361,7 +389,7 @@ sub process { next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); #trailing whitespace - if ($line=~/\+.*\S\s+$/) { + if ($line=~/^\+.*\S\s+$/) { my $herevet = "$here\n" . cat_vet($line) . "\n\n"; print "trailing whitespace\n"; print "$herevet"; @@ -392,17 +420,20 @@ sub process { # next if ($in_comment); - # Remove comments from the line before processing. +# Remove comments from the line before processing. $line =~ s@/\*.*\*/@@g; $line =~ s@/\*.*@@; $line =~ s@.*\*/@@; - # - # Checks which may be anchored in the context. - # +# Standardise the strings and chars within the input to simplify matching. + $line = sanitise_line($line); + +# +# Checks which may be anchored in the context. +# - # Check for switch () and associated case and default - # statements should be at the same indent. +# Check for switch () and associated case and default +# statements should be at the same indent. if ($line=~/\bswitch\s*\(.*\)/) { my $err = ''; my $sep = ''; @@ -428,9 +459,30 @@ sub process { #ignore lines not being added if ($line=~/^[^\+]/) {next;} - # - # Checks which are anchored on the added line. - # +# TEST: allow direct testing of the type matcher. + if ($tst_type && $line =~ /^.$Declare$/) { + print "TEST: is type $Declare\n"; + print "$herecurr"; + $clean = 0; + next; + } + +# +# Checks which are anchored on the added line. +# + +# check for malformed paths in #include statements (uses RAW line) + if ($rawline =~ m{^.#\s*include\s+[<"](.*)[">]}) { + my $path = $1; + if ($path =~ m{//}) { + print "malformed #include filename\n"; + print "$herecurr"; + $clean = 0; + } + # Sanitise this special form of string. + $path = 'X' x length($path); + $line =~ s{\<.*\>}{<$path>}; + } # no C99 // comments if ($line =~ m{//}) { @@ -441,47 +493,44 @@ sub process { # Remove C99 comments. $line =~ s@//.*@@; - # Standardise the strings and chars within the input - # to simplify matching. - $line = sanitise_line($line); - #EXPORT_SYMBOL should immediately follow its function closing }. - if (($line =~ /EXPORT_SYMBOL.*\(.*\)/) || - ($line =~ /EXPORT_UNUSED_SYMBOL.*\(.*\)/)) { + if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) || + ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { + my $name = $1; if (($prevline !~ /^}/) && ($prevline !~ /^\+}/) && - ($prevline !~ /^ }/)) { - print "EXPORT_SYMBOL(func); should immediately follow its function\n"; + ($prevline !~ /^ }/) && + ($prevline !~ /\s$name(?:\s+$Attribute)?\s*(?:;|=)/)) { + print "EXPORT_SYMBOL(foo); should immediately follow its function/variable\n"; print "$herecurr"; $clean = 0; } } - # check for static initialisers. +# check for static initialisers. if ($line=~/\s*static\s.*=\s+(0|NULL);/) { print "do not initialise statics to 0 or NULL\n"; print "$herecurr"; $clean = 0; } - # check for new typedefs. - if ($line=~/\s*typedef\s/) { +# check for new typedefs, only function parameters and sparse annotations +# make sense. + if ($line =~ /\btypedef\s/ && + $line !~ /\btypedef\s+$Type\s+\(\s*$Ident\s*\)\s*\(/ && + $line !~ /\b__bitwise(?:__|)\b/) { print "do not add new typedefs\n"; print "$herecurr"; $clean = 0; } # * goes on variable not on type - my $type = '(?:char|short|int|long|unsigned|float|double|' . - 'struct\s+[A-Za-z\d_]+|' . - 'union\s+[A-Za-z\d_]+)'; - if ($line =~ m{[A-Za-z\d_]+(\*+) [A-Za-z\d_]+}) { print "\"foo$1 bar\" should be \"foo $1bar\"\n"; print "$herecurr"; $clean = 0; } - if ($line =~ m{$type (\*) [A-Za-z\d_]+} || + if ($line =~ m{$Type (\*) [A-Za-z\d_]+} || $line =~ m{[A-Za-z\d_]+ (\*\*+) [A-Za-z\d_]+}) { print "\"foo $1 bar\" should be \"foo $1bar\"\n"; print "$herecurr"; @@ -530,13 +579,16 @@ sub process { } } -#function brace can't be on same line, except for #defines of do while, or if closed on same line +# function brace can't be on same line, except for #defines of do while, +# or if closed on same line if (($line=~/[A-Za-z\d_]+\**\s+\**[A-Za-z\d_]+\(.*\).* {/) and !($line=~/\#define.*do\s{/) and !($line=~/}/)) { print "braces following function declarations go on the next line\n"; print "$herecurr"; $clean = 0; } + +# Check operator spacing. # Note we expand the line with the leading + as the real # line will be displayed with the leading + and the tabs # will therefore also expand that way. @@ -544,7 +596,6 @@ sub process { $opline = expand_tabs($opline); $opline =~ s/^./ /; if (!($line=~/\#\s*include/)) { - # Check operator spacing. my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline); my $off = 0; for (my $n = 0; $n < $#elements; $n += 2) { @@ -572,8 +623,8 @@ sub process { # Pick up the preceeding and succeeding characters. my $ca = substr($opline, $off - 1, 1); my $cc = ''; - if (length($opline) > ($off + length($elements[$n]))) { - $cc = substr($opline, $off + 1 + length($elements[$n]), 1); + if (length($opline) >= ($off + length($elements[$n + 1]))) { + $cc = substr($opline, $off + length($elements[$n + 1]), 1); } my $ctx = "${a}x${c}"; @@ -598,7 +649,7 @@ sub process { # , must have a space on the right. } elsif ($op eq ',') { - if ($ctx !~ /.xW|.xE/) { + if ($ctx !~ /.xW|.xE/ && $cc ne '}') { print "need space after that '$op' $at\n"; print "$hereptr"; $clean = 0; @@ -619,11 +670,16 @@ sub process { # unary ++ and unary -- are allowed no space on one side. } elsif ($op eq '++' or $op eq '--') { - if ($ctx !~ /[WOB]x[^W]|[^W]x[WOB]/) { + if ($ctx !~ /[WOB]x[^W]/ && $ctx !~ /[^W]x[WOB]/) { print "need space one side of that '$op' $at\n"; print "$hereptr"; $clean = 0; } + if ($ctx =~ /Wx./ && $cc eq ';') { + print "no space before that '$op' $at\n"; + print "$hereptr"; + $clean = 0; + } # & is both unary and binary # unary: @@ -656,7 +712,7 @@ sub process { print "$hereptr"; $clean = 0; } - } elsif ($ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]|OxV|WxB/) { + } elsif ($ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]|OxV|WxB|BxB/) { print "need space before that '$op' $at\n"; print "$hereptr"; $clean = 0; @@ -705,9 +761,9 @@ sub process { } # Check for illegal assignment in if conditional. - if ($line=~/\b(if|while)\s*\(.*[^<>!=]=[^=].*\)/) { + if ($line=~/\bif\s*\(.*[^<>!=]=[^=].*\)/) { #next if ($line=~/\".*\Q$op\E.*\"/ or $line=~/\'\Q$op\E\'/); - print "do not use assignment in condition\n"; + print "do not use assignment in if condition\n"; print "$herecurr"; $clean = 0; } @@ -735,8 +791,8 @@ sub process { $clean = 0; } -#warn if is #included and is available. - if ($tree && $line =~ qr|\s*\#\s*include\s*\|) { +#warn if is #included and is available (uses RAW line) + if ($tree && $rawline =~ m{^.\#\s*include\s*\}) { my $checkfile = "include/linux/$1.h"; if (-f $checkfile) { print "Use #include instead of \n"; @@ -745,7 +801,8 @@ sub process { } } -#if/while/etc brace do not go on next line, unless #defining a do while loop, or if that brace on the next line is for something else +# if/while/etc brace do not go on next line, unless defining a do while loop, +# or if that brace on the next line is for something else if ($prevline=~/\b(if|while|for|switch)\s*\(/) { my @opened = $prevline=~/\(/g; my @closed = $prevline=~/\)/g; @@ -767,25 +824,36 @@ sub process { } if (($prevline=~/\b(if|while|for|switch)\s*\(.*\)\s*$/) and ($next_line=~/{/) and - !($next_line=~/\b(if|while|for)/) and !($next_line=~/\#define.*do.*while/)) { + !($next_line=~/\b(if|while|for|switch)/) and !($next_line=~/\#define.*do.*while/)) { print "That { should be on the previous line\n"; print "$here\n$display_segment\n$next_line\n\n"; $clean = 0; } } -#multiline macros should be enclosed in a do while loop - if (($prevline=~/\#define.*\\/) and !($prevline=~/do\s+{/) and - !($prevline=~/\(\{/) and ($line=~/;\s*\\/) and - !($line=~/do.*{/) and !($line=~/\(\{/)) { - print "Macros with multiple statements should be enclosed in a do - while loop\n"; - print "$hereprev"; - $clean = 0; +# multi-statement macros should be enclosed in a do while loop, grab the +# first statement and ensure its the whole macro if its not enclosed +# in a known goot container + if (($prevline=~/\#define.*\\/) and + !($prevline=~/do\s+{/) and !($prevline=~/\(\{/) and + !($line=~/do.*{/) and !($line=~/\(\{/) and + !($line=~/^.\s*$Declare\s/)) { + # Grab the first statement, if that is the entire macro + # its ok. This may start either on the #define line + # or the one below. + my $ctx1 = join('', ctx_statement($linenr - 1, $realcnt + 1)); + my $ctx2 = join('', ctx_statement($linenr, $realcnt)); + + if ($ctx1 =~ /\\$/ && $ctx2 =~ /\\$/) { + print "Macros with multiple statements should be enclosed in a do - while loop\n"; + print "$hereprev"; + $clean = 0; + } } -# don't include deprecated include files +# don't include deprecated include files (uses RAW line) for my $inc (@dep_includes) { - if ($line =~ m@\#\s*include\s*\<$inc>@) { + if ($rawline =~ m@\#\s*include\s*\<$inc>@) { print "Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n"; print "$herecurr"; $clean = 0; @@ -845,6 +913,13 @@ sub process { print "$herecurr"; $clean = 0; } + + if ($line =~ /$Type\s+(?:inline|__always_inline)\b/ || + $line =~ /\b(?:inline|always_inline)\s+$Storage/) { + print "inline keyword should sit between storage class and type\n"; + print "$herecurr"; + $clean = 0; + } } if ($chk_patch && !$is_patch) { -- cgit v1.2.3 From d8aaf12142d066d3982475d58a9094c85a06a5a9 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Sat, 23 Jun 2007 17:16:44 -0700 Subject: update checkpatch.pl to version 0.06 Update to checkpatch.pl v0.06. Of note: - do { and else handled correctly as control structures for { matching - trailing whitespace correctly tripped when line otherwise empty - support for const, including const foo * const bar - multiline macros defining values correctly reported This version of checkpatch.pl can be found at the following URL: http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-0.06 Full Changelog: Andy Whitcroft (14): Version: 0.06 cleanup the Type regular expression declarations fix up block counting end of line counts as a space for ++ and -- do { needs the same checks as if, for et al handle "const foo * const a" as a valid type add spacing checks following ; complete whitespace lines should trip trailing whitespace check else is also a block control structure badly formatted else can trip function declaration detect and report trailing statements after else types need to be terminated by a boundary multiline macros defining values should be surrounded by parentheses soften the wording of the Signed-off-by: warnings Signed-off-by: Andy Whitcroft Cc: "Randy.Dunlap" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 145 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 92 insertions(+), 53 deletions(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 56c364c1df81..277c32647f36 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,7 +9,7 @@ use strict; my $P = $0; $P =~ s@.*/@@g; -my $V = '0.05'; +my $V = '0.06'; use Getopt::Long qw(:config no_auto_abbrev); @@ -271,24 +271,38 @@ sub process { my $in_comment = 0; my $first_line = 0; - my $ident = '[A-Za-z\d_]+'; - my $storage = '(?:extern|static)'; - my $sparse = '(?:__user|__kernel|__force|__iomem)'; - my $type = '(?:unsigned\s+)?' . - '(?:void|char|short|int|long|unsigned|float|double|' . - 'long\s+long|' . - "struct\\s+${ident}|" . - "union\\s+${ident}|" . - "${ident}_t)" . - "(?:\\s+$sparse)*" . - '(?:\s*\*+)?'; - my $attribute = '(?:__read_mostly|__init|__initdata)'; - - my $Ident = $ident; - my $Type = $type; - my $Storage = $storage; - my $Declare = "(?:$storage\\s+)?$type"; - my $Attribute = $attribute; + my $Ident = qr{[A-Za-z\d_]+}; + my $Storage = qr{extern|static}; + my $Sparse = qr{__user|__kernel|__force|__iomem}; + my $NonptrType = qr{ + \b + (?:const\s+)? + (?:unsigned\s+)? + (?: + void| + char| + short| + int| + long| + unsigned| + float| + double| + long\s+int| + long\s+long| + long\s+long\s+int| + struct\s+$Ident| + union\s+$Ident| + ${Ident}_t + ) + (?:\s+$Sparse)* + \b + }x; + my $Type = qr{ + \b$NonptrType\b + (?:\s*\*+\s*const|\s*\*+)? + }x; + my $Declare = qr{(?:$Storage\s+)?$Type}; + my $Attribute = qr{__read_mostly|__init|__initdata}; foreach my $line (@lines) { $linenr++; @@ -321,6 +335,7 @@ sub process { # blank context lines so we need to count that too. if ($line =~ /^( |\+|$)/) { $realline++; + $realcnt-- if ($realcnt != 0); # track any sort of multi-line comment. Obviously if # the added text or context do not include the whole @@ -345,8 +360,9 @@ sub process { # Track the previous line. ($prevline, $stashline) = ($stashline, $line); ($previndent, $stashindent) = ($stashindent, $indent); + } elsif ($realcnt == 1) { + $realcnt--; } - $realcnt-- if ($realcnt != 0); #make up the handle for any error we report on this line $here = "#$linenr: "; @@ -357,14 +373,11 @@ sub process { my $hereprev = "$here\n$prevline\n$line\n\n"; #check the patch for a signoff: - if ($line =~ /^\s*Signed-off-by:\s/) { - $signoff++; - - } elsif ($line =~ /^\s*signed-off-by:/i) { + if ($line =~ /^\s*signed-off-by:/i) { # This is a signoff, if ugly, so do not double report. $signoff++; if (!($line =~ /^\s*Signed-off-by:/)) { - print "use Signed-off-by:\n"; + print "Signed-off-by: is the preferred form\n"; print "$herecurr"; $clean = 0; } @@ -389,7 +402,7 @@ sub process { next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); #trailing whitespace - if ($line=~/^\+.*\S\s+$/) { + if ($line =~ /^\+.*\S\s+$/ || $line =~ /^\+\s+$/) { my $herevet = "$here\n" . cat_vet($line) . "\n\n"; print "trailing whitespace\n"; print "$herevet"; @@ -525,24 +538,23 @@ sub process { } # * goes on variable not on type - if ($line =~ m{[A-Za-z\d_]+(\*+) [A-Za-z\d_]+}) { - print "\"foo$1 bar\" should be \"foo $1bar\"\n"; + if ($line =~ m{\($NonptrType(\*+)(?:\s+const)?\)}) { + print "\"(foo$1)\" should be \"(foo $1)\"\n"; print "$herecurr"; $clean = 0; - } - if ($line =~ m{$Type (\*) [A-Za-z\d_]+} || - $line =~ m{[A-Za-z\d_]+ (\*\*+) [A-Za-z\d_]+}) { - print "\"foo $1 bar\" should be \"foo $1bar\"\n"; + + } elsif ($line =~ m{\($NonptrType\s+(\*+)(?!\s+const)\s+\)}) { + print "\"(foo $1 )\" should be \"(foo $1)\"\n"; print "$herecurr"; $clean = 0; - } - if ($line =~ m{\([A-Za-z\d_\s]+[A-Za-z\d_](\*+)\)}) { - print "\"(foo$1)\" should be \"(foo $1)\"\n"; + + } elsif ($line =~ m{$NonptrType(\*+)(?:\s+const)?\s+[A-Za-z\d_]+}) { + print "\"foo$1 bar\" should be \"foo $1bar\"\n"; print "$herecurr"; $clean = 0; - } - if ($line =~ m{\([A-Za-z\d_\s]+[A-Za-z\d_]\s+(\*+)\s+\)}) { - print "\"(foo $1 )\" should be \"(foo $1)\"\n"; + + } elsif ($line =~ m{$NonptrType\s+(\*+)(?!\s+const)\s+[A-Za-z\d_]+}) { + print "\"foo $1 bar\" should be \"foo $1bar\"\n"; print "$herecurr"; $clean = 0; } @@ -581,7 +593,7 @@ sub process { # function brace can't be on same line, except for #defines of do while, # or if closed on same line - if (($line=~/[A-Za-z\d_]+\**\s+\**[A-Za-z\d_]+\(.*\).* {/) and + if (($line=~/$Type\s*[A-Za-z\d_]+\(.*\).* {/) and !($line=~/\#define.*do\s{/) and !($line=~/}/)) { print "braces following function declarations go on the next line\n"; print "$herecurr"; @@ -624,7 +636,7 @@ sub process { my $ca = substr($opline, $off - 1, 1); my $cc = ''; if (length($opline) >= ($off + length($elements[$n + 1]))) { - $cc = substr($opline, $off + length($elements[$n + 1]), 1); + $cc = substr($opline, $off + length($elements[$n + 1])); } my $ctx = "${a}x${c}"; @@ -636,8 +648,16 @@ sub process { ##print "<$s1:$op:$s2> <$elements[$n]:$elements[$n + 1]:$elements[$n + 2]>\n"; - # We need ; as an operator. // is a comment. - if ($op eq ';' or $op eq '//') { + # ; should have either the end of line or a space or \ after it + if ($op eq ';') { + if ($ctx !~ /.x[WE]/ && $cc !~ /^\\/) { + print "need space after that '$op' $at\n"; + print "$hereptr"; + $clean = 0; + } + + # // is a comment + } elsif ($op eq '//') { # -> should have no spaces } elsif ($op eq '->') { @@ -649,7 +669,7 @@ sub process { # , must have a space on the right. } elsif ($op eq ',') { - if ($ctx !~ /.xW|.xE/ && $cc ne '}') { + if ($ctx !~ /.xW|.xE/ && $cc !~ /^}/) { print "need space after that '$op' $at\n"; print "$hereptr"; $clean = 0; @@ -670,12 +690,12 @@ sub process { # unary ++ and unary -- are allowed no space on one side. } elsif ($op eq '++' or $op eq '--') { - if ($ctx !~ /[WOB]x[^W]/ && $ctx !~ /[^W]x[WOB]/) { + if ($ctx !~ /[WOB]x[^W]/ && $ctx !~ /[^W]x[WOBE]/) { print "need space one side of that '$op' $at\n"; print "$hereptr"; $clean = 0; } - if ($ctx =~ /Wx./ && $cc eq ';') { + if ($ctx =~ /Wx./ && $cc =~ /^;/) { print "no space before that '$op' $at\n"; print "$hereptr"; $clean = 0; @@ -707,7 +727,7 @@ sub process { # } elsif ($op eq '*') { if ($ca eq '*') { - if ($cc =~ /\s/) { + if ($cc =~ /^\s(?!\s*const)/) { print "no space after that '$op' $at\n"; print "$hereptr"; $clean = 0; @@ -803,7 +823,7 @@ sub process { # if/while/etc brace do not go on next line, unless defining a do while loop, # or if that brace on the next line is for something else - if ($prevline=~/\b(if|while|for|switch)\s*\(/) { + if ($prevline=~/\b(?:(if|while|for|switch)\s*\(|do\b|else\b)/) { my @opened = $prevline=~/\(/g; my @closed = $prevline=~/\)/g; my $nr_line = $linenr; @@ -823,14 +843,22 @@ sub process { @closed = $prevline=~/\)/g; } - if (($prevline=~/\b(if|while|for|switch)\s*\(.*\)\s*$/) and ($next_line=~/{/) and - !($next_line=~/\b(if|while|for|switch)/) and !($next_line=~/\#define.*do.*while/)) { + if (($prevline=~/\b(?:(if|while|for|switch)\s*\(.*\)|do|else)\s*$/) and ($next_line=~/{/) and + !($next_line=~/\b(?:if|while|for|switch|do|else)\b/) and !($next_line=~/\#define.*do.*while/)) { print "That { should be on the previous line\n"; print "$here\n$display_segment\n$next_line\n\n"; $clean = 0; } } +# if and else should not have general statements after it + if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ && + $1 !~ /^\s*(?:\sif|{|$)/) { + print "trailing statements should be on next line\n"; + print "$herecurr"; + $clean = 0; + } + # multi-statement macros should be enclosed in a do while loop, grab the # first statement and ensure its the whole macro if its not enclosed # in a known goot container @@ -841,11 +869,22 @@ sub process { # Grab the first statement, if that is the entire macro # its ok. This may start either on the #define line # or the one below. - my $ctx1 = join('', ctx_statement($linenr - 1, $realcnt + 1)); - my $ctx2 = join('', ctx_statement($linenr, $realcnt)); + my $ln = $linenr; + my $cnt = $realcnt; - if ($ctx1 =~ /\\$/ && $ctx2 =~ /\\$/) { - print "Macros with multiple statements should be enclosed in a do - while loop\n"; + # If the macro starts on the define line start there. + if ($prevline !~ m{^.#\s*define\s*$Ident(?:\([^\)]*\))?\s*\\\s*$}) { + $ln--; + $cnt++; + } + my $ctx = join('', ctx_statement($ln, $cnt)); + + if ($ctx =~ /\\$/) { + if ($ctx =~ /;/) { + print "Macros with multiple statements should be enclosed in a do - while loop\n"; + } else { + print "Macros with complex values should be enclosed in parenthesis\n"; + } print "$hereprev"; $clean = 0; } -- cgit v1.2.3 From edd5cd4a9424f22b0fa08bef5e299d41befd5622 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 27 Jun 2007 14:10:09 -0700 Subject: Introduce fixed sys_sync_file_range2() syscall, implement on PowerPC and ARM Not all the world is an i386. Many architectures need 64-bit arguments to be aligned in suitable pairs of registers, and the original sys_sync_file_range(int, loff_t, loff_t, int) was therefore wasting an argument register for padding after the first integer. Since we don't normally have more than 6 arguments for system calls, that left no room for the final argument on some architectures. Fix this by introducing sys_sync_file_range2(int, int, loff_t, loff_t) which all fits nicely. In fact, ARM already had that, but called it sys_arm_sync_file_range. Move it to fs/sync.c and rename it, then implement the needed compatibility routine. And stop the missing syscall check from bitching about the absence of sys_sync_file_range() if we've implemented sys_sync_file_range2() instead. Tested on PPC32 and with 32-bit and 64-bit userspace on PPC64. Signed-off-by: David Woodhouse Acked-by: Russell King Cc: Arnd Bergmann Cc: Paul Mackerras Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- arch/arm/kernel/calls.S | 2 +- arch/arm/kernel/sys_arm.c | 13 ------------- arch/powerpc/kernel/sys_ppc32.c | 9 +++++++++ fs/sync.c | 8 ++++++++ include/asm-arm/unistd.h | 1 + include/asm-powerpc/systbl.h | 1 + include/asm-powerpc/unistd.h | 3 ++- include/linux/syscalls.h | 2 ++ scripts/checksyscalls.sh | 5 +++++ 9 files changed, 29 insertions(+), 15 deletions(-) (limited to 'scripts') diff --git a/arch/arm/kernel/calls.S b/arch/arm/kernel/calls.S index 19326d7cdeb3..a98d0c933db0 100644 --- a/arch/arm/kernel/calls.S +++ b/arch/arm/kernel/calls.S @@ -350,7 +350,7 @@ CALL(sys_set_robust_list) CALL(sys_get_robust_list) /* 340 */ CALL(sys_splice) - CALL(sys_arm_sync_file_range) + CALL(sys_sync_file_range2) CALL(sys_tee) CALL(sys_vmsplice) CALL(sys_move_pages) diff --git a/arch/arm/kernel/sys_arm.c b/arch/arm/kernel/sys_arm.c index 1ca2d5174fcb..4d25e49a14f7 100644 --- a/arch/arm/kernel/sys_arm.c +++ b/arch/arm/kernel/sys_arm.c @@ -328,16 +328,3 @@ asmlinkage long sys_arm_fadvise64_64(int fd, int advice, { return sys_fadvise64_64(fd, offset, len, advice); } - -/* - * Yet more syscall fsckage - we can't fit sys_sync_file_range's - * arguments into the available registers with EABI. So, let's - * create an ARM specific syscall for this which has _sane_ - * arguments. (This incidentally also has an ABI-independent - * argument layout.) - */ -asmlinkage long sys_arm_sync_file_range(int fd, unsigned int flags, - loff_t offset, loff_t nbytes) -{ - return sys_sync_file_range(fd, offset, nbytes, flags); -} diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c index 047246ad4f65..b42cbf1e2d7d 100644 --- a/arch/powerpc/kernel/sys_ppc32.c +++ b/arch/powerpc/kernel/sys_ppc32.c @@ -810,3 +810,12 @@ asmlinkage long compat_sys_request_key(const char __user *_type, return sys_request_key(_type, _description, _callout_info, destringid); } +asmlinkage long compat_sys_sync_file_range2(int fd, unsigned int flags, + unsigned offset_hi, unsigned offset_lo, + unsigned nbytes_hi, unsigned nbytes_lo) +{ + loff_t offset = ((loff_t)offset_hi << 32) | offset_lo; + loff_t nbytes = ((loff_t)nbytes_hi << 32) | nbytes_lo; + + return sys_sync_file_range(fd, offset, nbytes, flags); +} diff --git a/fs/sync.c b/fs/sync.c index 2f97576355b8..7cd005ea7639 100644 --- a/fs/sync.c +++ b/fs/sync.c @@ -236,6 +236,14 @@ out: return ret; } +/* It would be nice if people remember that not all the world's an i386 + when they introduce new system calls */ +asmlinkage long sys_sync_file_range2(int fd, unsigned int flags, + loff_t offset, loff_t nbytes) +{ + return sys_sync_file_range(fd, offset, nbytes, flags); +} + /* * `endbyte' is inclusive */ diff --git a/include/asm-arm/unistd.h b/include/asm-arm/unistd.h index 250d7f145aca..bfdbebebdc1b 100644 --- a/include/asm-arm/unistd.h +++ b/include/asm-arm/unistd.h @@ -367,6 +367,7 @@ #define __NR_get_robust_list (__NR_SYSCALL_BASE+339) #define __NR_splice (__NR_SYSCALL_BASE+340) #define __NR_arm_sync_file_range (__NR_SYSCALL_BASE+341) +#define __NR_sync_file_range2 __NR_arm_sync_file_range #define __NR_tee (__NR_SYSCALL_BASE+342) #define __NR_vmsplice (__NR_SYSCALL_BASE+343) #define __NR_move_pages (__NR_SYSCALL_BASE+344) diff --git a/include/asm-powerpc/systbl.h b/include/asm-powerpc/systbl.h index 700ca5928741..1cc3f9cb6f4e 100644 --- a/include/asm-powerpc/systbl.h +++ b/include/asm-powerpc/systbl.h @@ -311,3 +311,4 @@ COMPAT_SYS_SPU(utimensat) COMPAT_SYS_SPU(signalfd) COMPAT_SYS_SPU(timerfd) SYSCALL_SPU(eventfd) +COMPAT_SYS_SPU(sync_file_range2) diff --git a/include/asm-powerpc/unistd.h b/include/asm-powerpc/unistd.h index e3c28dc31abf..f71c6061f1ec 100644 --- a/include/asm-powerpc/unistd.h +++ b/include/asm-powerpc/unistd.h @@ -330,10 +330,11 @@ #define __NR_signalfd 305 #define __NR_timerfd 306 #define __NR_eventfd 307 +#define __NR_sync_file_range2 308 #ifdef __KERNEL__ -#define __NR_syscalls 308 +#define __NR_syscalls 309 #define __NR__exit __NR_exit #define NR_syscalls __NR_syscalls diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index b02070eac422..83d0ec11235e 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -598,6 +598,8 @@ asmlinkage long sys_tee(int fdin, int fdout, size_t len, unsigned int flags); asmlinkage long sys_sync_file_range(int fd, loff_t offset, loff_t nbytes, unsigned int flags); +asmlinkage long sys_sync_file_range2(int fd, unsigned int flags, + loff_t offset, loff_t nbytes); asmlinkage long sys_get_robust_list(int pid, struct robust_list_head __user * __user *head_ptr, size_t __user *len_ptr); diff --git a/scripts/checksyscalls.sh b/scripts/checksyscalls.sh index f98171f5a3df..0dcc01ce45a6 100755 --- a/scripts/checksyscalls.sh +++ b/scripts/checksyscalls.sh @@ -99,6 +99,11 @@ cat << EOF #define __IGNORE_setfsuid32 #define __IGNORE_setfsgid32 +/* sync_file_range had a stupid ABI. Allow sync_file_range2 instead */ +#ifdef __NR_sync_file_range2 +#define __IGNORE_sync_file_range +#endif + /* Unmerged syscalls for AFS, STREAMS, etc. */ #define __IGNORE_afs_syscall #define __IGNORE_getpmsg -- cgit v1.2.3 From 0db19c412ce260a293b06b4bab66550b84411bfc Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Fri, 6 Jul 2007 02:39:52 -0700 Subject: x86_64: fix headers_install A bug in headers_install for ARCH=x86_64 yields an asm/ directory full of files all of which are using the same #ifdef guard, "__ASM_STUB_" with no postfix. So the second and later asm files #included in the same C file (often through standard headers like ioctl.h) yields no symbols. Strangeness with the Ubuntu 'tell me if I support something that's not explcitly mentioned in POSIX, and I'll strip it out' shell, I believe. We don't need the 'export' but we do need a semicolon at the end of the FNAME line: Signed-off-by: David Woodhouse Signed-off-by: Rob Landley Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/Makefile.headersinst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts') diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst index 8cd63014a0d1..f98d772aac80 100644 --- a/scripts/Makefile.headersinst +++ b/scripts/Makefile.headersinst @@ -108,7 +108,7 @@ quiet_cmd_mkdir = MKDIR $(patsubst $(INSTALL_HDR_PATH)/%,%,$@) quiet_cmd_gen = GEN $(patsubst $(INSTALL_HDR_PATH)/%,%,$@) cmd_gen = \ -FNAME=$(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$@) \ +FNAME=$(patsubst $(INSTALL_HDR_PATH)/$(_dst)/%,%,$@); \ STUBDEF=__ASM_STUB_`echo $$FNAME | tr a-z.- A-Z__`; \ (echo "/* File autogenerated by 'make headers_install' */" ; \ echo "\#ifndef $$STUBDEF" ; \ -- cgit v1.2.3 From de7d4f0e1172a72277d660fa0be59654ea02bed0 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Sun, 15 Jul 2007 23:37:22 -0700 Subject: update checkpatch.pl to version 0.07 This version brings a number of new checks, fixes for flase positives, plus a clarification of the output to better guide use. Of note: - checks for documentation for new __setup calls - clearer reporting where braces and parenthesis are involved - reports for closing brace and semi-colon spacing - reports on unwanted externs This patch includes an update to the documentation on checkpatch.pl itself to clarify when it should be used and to indicate that it is not intended as the final arbitor of style. Full changelog: Andy Whitcroft (19): Version: 0.07 ensure we do not apply control brace checks to preprocesor directives add {u,s}{8,16,32,64} to the type matcher accept lack of spacing after the semicolons in for (;;) report new externs in .c files fix up typedef exclusion for function prototypes else trailing statements check need to account for \ at end of line add enums to the type matcher add missing check descriptions suppress double reporting of ** spacing report on do{ spacing issues include an example of the brace/parenthesis in output check for spacing after closing braces prevent double reports on pointer spacing issues handle blank continuation lines on macros classify all reports error, warning, or check revamp hanging { checks and apply in context no spaces after the last ; in a for is ok check __setup has a corresponding addition to documentation David Woodhouse (1): limit character set used in patches and descriptions to UTF-8 Signed-off-by: Andy Whitcroft Cc: David Woodhouse Cc: "Randy.Dunlap" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- Documentation/SubmittingPatches | 20 ++- scripts/checkpatch.pl | 374 +++++++++++++++++++--------------------- 2 files changed, 196 insertions(+), 198 deletions(-) (limited to 'scripts') diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 0958e97d4bf4..3f9a7912e69b 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -464,9 +464,25 @@ section Linus Computer Science 101. Nuff said. If your code deviates too much from this, it is likely to be rejected without further review, and without comment. +Once significant exception is when moving code from one file to +another in this case you should not modify the moved code at all in +the same patch which moves it. This clearly delineates the act of +moving the code and your changes. This greatly aids review of the +actual differences and allows tools to better track the history of +the code itself. + Check your patches with the patch style checker prior to submission -(scripts/checkpatch.pl). You should be able to justify all -violations that remain in your patch. +(scripts/checkpatch.pl). The style checker should be viewed as +a guide not as the final word. If your code looks better with +a violation then its probably best left alone. + +The checker reports at three levels: + - ERROR: things that are very likely to be wrong + - WARNING: things requiring careful review + - CHECK: things requiring thought + +You should be able to justify all violations that remain in your +patch. diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 277c32647f36..25e20a27fc59 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,7 +9,7 @@ use strict; my $P = $0; $P =~ s@.*/@@g; -my $V = '0.06'; +my $V = '0.07'; use Getopt::Long qw(:config no_auto_abbrev); @@ -246,6 +246,19 @@ sub cat_vet { return $vet; } +sub ERROR { + print "ERROR: $_[0]\n"; + our $clean = 0; +} +sub WARN { + print "WARNING: $_[0]\n"; + our $clean = 0; +} +sub CHK { + print "CHECK: $_[0]\n"; + our $clean = 0; +} + sub process { my $filename = shift; my @lines = @_; @@ -259,7 +272,7 @@ sub process { my $previndent=0; my $stashindent=0; - my $clean = 1; + our $clean = 1; my $signoff = 0; my $is_patch = 0; @@ -290,8 +303,11 @@ sub process { long\s+int| long\s+long| long\s+long\s+int| + u8|u16|u32|u64| + s8|s16|s32|s64| struct\s+$Ident| union\s+$Ident| + enum\s+$Ident| ${Ident}_t ) (?:\s+$Sparse)* @@ -304,6 +320,23 @@ sub process { my $Declare = qr{(?:$Storage\s+)?$Type}; my $Attribute = qr{__read_mostly|__init|__initdata}; + # Pre-scan the patch looking for any __setup documentation. + my @setup_docs = (); + my $setup_docs = 0; + foreach my $line (@lines) { + if ($line=~/^\+\+\+\s+(\S+)/) { + $setup_docs = 0; + if ($1 =~ m@Documentation/kernel-parameters.txt$@) { + $setup_docs = 1; + } + next; + } + + if ($setup_docs && $line =~ /^\+/) { + push(@setup_docs, $line); + } + } + foreach my $line (@lines) { $linenr++; @@ -369,30 +402,42 @@ sub process { $here .= "FILE: $realfile:$realline:" if ($realcnt != 0); my $hereline = "$here\n$line\n"; - my $herecurr = "$here\n$line\n\n"; - my $hereprev = "$here\n$prevline\n$line\n\n"; + my $herecurr = "$here\n$line\n"; + my $hereprev = "$here\n$prevline\n$line\n"; #check the patch for a signoff: if ($line =~ /^\s*signed-off-by:/i) { # This is a signoff, if ugly, so do not double report. $signoff++; if (!($line =~ /^\s*Signed-off-by:/)) { - print "Signed-off-by: is the preferred form\n"; - print "$herecurr"; - $clean = 0; + WARN("Signed-off-by: is the preferred form\n" . + $herecurr); } if ($line =~ /^\s*signed-off-by:\S/i) { - print "need space after Signed-off-by:\n"; - print "$herecurr"; - $clean = 0; + WARN("need space after Signed-off-by:\n" . + $herecurr); } } # Check for wrappage within a valid hunk of the file if ($realcnt != 0 && $line !~ m{^(?:\+|-| |$)}) { - print "patch seems to be corrupt (line wrapped?) [$realcnt]\n"; - print "$herecurr"; - $clean = 0; + ERROR("patch seems to be corrupt (line wrapped?)\n" . + $herecurr); + } + +# UTF-8 regex found at http://www.w3.org/International/questions/qa-forms-utf-8.en.php + if (($realfile =~ /^$/ || $line =~ /^\+/) && + !($line =~ m/^( + [\x09\x0A\x0D\x20-\x7E] # ASCII + | [\xC2-\xDF][\x80-\xBF] # non-overlong 2-byte + | \xE0[\xA0-\xBF][\x80-\xBF] # excluding overlongs + | [\xE1-\xEC\xEE\xEF][\x80-\xBF]{2} # straight 3-byte + | \xED[\x80-\x9F][\x80-\xBF] # excluding surrogates + | \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3 + | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15 + | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 + )*$/x )) { + ERROR("Invalid UTF-8\n" . $herecurr); } #ignore lines being removed @@ -403,16 +448,12 @@ sub process { #trailing whitespace if ($line =~ /^\+.*\S\s+$/ || $line =~ /^\+\s+$/) { - my $herevet = "$here\n" . cat_vet($line) . "\n\n"; - print "trailing whitespace\n"; - print "$herevet"; - $clean = 0; + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("trailing whitespace\n" . $herevet); } #80 column limit if ($line =~ /^\+/ && !($prevline=~/\/\*\*/) && $length > 80) { - print "line over 80 characters\n"; - print "$herecurr"; - $clean = 0; + WARN("line over 80 characters\n" . $herecurr); } # check we are in a valid source file *.[hc] if not then ignore this hunk @@ -421,10 +462,8 @@ sub process { # at the beginning of a line any tabs must come first and anything # more than 8 must use tabs. if ($line=~/^\+\s* \t\s*\S/ or $line=~/^\+\s* \s*/) { - my $herevet = "$here\n" . cat_vet($line) . "\n\n"; - print "use tabs not spaces\n"; - print "$herevet"; - $clean = 0; + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("use tabs not spaces\n" . $herevet); } # @@ -463,9 +502,27 @@ sub process { } } if ($err ne '') { - print "switch and case should be at the same indent\n"; - print "$here\n$line\n$err\n"; - $clean = 0; + ERROR("switch and case should be at the same indent\n$hereline\n$err\n"); + } + } + +# if/while/etc brace do not go on next line, unless defining a do while loop, +# or if that brace on the next line is for something else + if ($line =~ /\b(?:(if|while|for|switch)\s*\(|do\b|else\b)/ && $line !~ /^.#/) { + my @ctx = ctx_statement($linenr, $realcnt); + my $ctx_ln = $linenr + $#ctx + 1; + my $ctx_cnt = $realcnt - $#ctx - 1; + my $ctx = join("\n", @ctx); + + while ($ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^-/) { + $ctx_ln++; + $ctx_cnt--; + } + ##warn "line<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>"; + + if ($ctx !~ /{\s*/ && $ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^\+\s*{/) { + ERROR("That { should be on the previous line\n" . + "$here\n$ctx\n$lines[$ctx_ln - 1]"); } } @@ -474,9 +531,7 @@ sub process { # TEST: allow direct testing of the type matcher. if ($tst_type && $line =~ /^.$Declare$/) { - print "TEST: is type $Declare\n"; - print "$herecurr"; - $clean = 0; + ERROR("TEST: is type $Declare\n" . $herecurr); next; } @@ -488,9 +543,8 @@ sub process { if ($rawline =~ m{^.#\s*include\s+[<"](.*)[">]}) { my $path = $1; if ($path =~ m{//}) { - print "malformed #include filename\n"; - print "$herecurr"; - $clean = 0; + ERROR("malformed #include filename\n" . + $herecurr); } # Sanitise this special form of string. $path = 'X' x length($path); @@ -499,9 +553,7 @@ sub process { # no C99 // comments if ($line =~ m{//}) { - print "do not use C99 // comments\n"; - print "$herecurr"; - $clean = 0; + ERROR("do not use C99 // comments\n" . $herecurr); } # Remove C99 comments. $line =~ s@//.*@@; @@ -514,49 +566,40 @@ sub process { ($prevline !~ /^\+}/) && ($prevline !~ /^ }/) && ($prevline !~ /\s$name(?:\s+$Attribute)?\s*(?:;|=)/)) { - print "EXPORT_SYMBOL(foo); should immediately follow its function/variable\n"; - print "$herecurr"; - $clean = 0; + WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr); } } # check for static initialisers. if ($line=~/\s*static\s.*=\s+(0|NULL);/) { - print "do not initialise statics to 0 or NULL\n"; - print "$herecurr"; - $clean = 0; + ERROR("do not initialise statics to 0 or NULL\n" . + $herecurr); } # check for new typedefs, only function parameters and sparse annotations # make sense. if ($line =~ /\btypedef\s/ && - $line !~ /\btypedef\s+$Type\s+\(\s*$Ident\s*\)\s*\(/ && + $line !~ /\btypedef\s+$Type\s+\(\s*\*$Ident\s*\)\s*\(/ && $line !~ /\b__bitwise(?:__|)\b/) { - print "do not add new typedefs\n"; - print "$herecurr"; - $clean = 0; + WARN("do not add new typedefs\n" . $herecurr); } # * goes on variable not on type if ($line =~ m{\($NonptrType(\*+)(?:\s+const)?\)}) { - print "\"(foo$1)\" should be \"(foo $1)\"\n"; - print "$herecurr"; - $clean = 0; + ERROR("\"(foo$1)\" should be \"(foo $1)\"\n" . + $herecurr); } elsif ($line =~ m{\($NonptrType\s+(\*+)(?!\s+const)\s+\)}) { - print "\"(foo $1 )\" should be \"(foo $1)\"\n"; - print "$herecurr"; - $clean = 0; + ERROR("\"(foo $1 )\" should be \"(foo $1)\"\n" . + $herecurr); } elsif ($line =~ m{$NonptrType(\*+)(?:\s+const)?\s+[A-Za-z\d_]+}) { - print "\"foo$1 bar\" should be \"foo $1bar\"\n"; - print "$herecurr"; - $clean = 0; + ERROR("\"foo$1 bar\" should be \"foo $1bar\"\n" . + $herecurr); } elsif ($line =~ m{$NonptrType\s+(\*+)(?!\s+const)\s+[A-Za-z\d_]+}) { - print "\"foo $1 bar\" should be \"foo $1bar\"\n"; - print "$herecurr"; - $clean = 0; + ERROR("\"foo $1 bar\" should be \"foo $1bar\"\n" . + $herecurr); } # # no BUG() or BUG_ON() @@ -585,9 +628,7 @@ sub process { } } if ($ok == 0) { - print "printk() should include KERN_ facility level\n"; - print "$herecurr"; - $clean = 0; + WARN("printk() should include KERN_ facility level\n" . $herecurr); } } @@ -595,9 +636,7 @@ sub process { # or if closed on same line if (($line=~/$Type\s*[A-Za-z\d_]+\(.*\).* {/) and !($line=~/\#define.*do\s{/) and !($line=~/}/)) { - print "braces following function declarations go on the next line\n"; - print "$herecurr"; - $clean = 0; + ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr); } # Check operator spacing. @@ -633,27 +672,27 @@ sub process { } # Pick up the preceeding and succeeding characters. - my $ca = substr($opline, $off - 1, 1); + my $ca = substr($opline, 0, $off); my $cc = ''; if (length($opline) >= ($off + length($elements[$n + 1]))) { $cc = substr($opline, $off + length($elements[$n + 1])); } + my $cb = "$ca$;$cc"; my $ctx = "${a}x${c}"; my $at = "(ctx:$ctx)"; my $ptr = (" " x $off) . "^"; - my $hereptr = "$hereline$ptr\n\n"; + my $hereptr = "$hereline$ptr\n"; ##print "<$s1:$op:$s2> <$elements[$n]:$elements[$n + 1]:$elements[$n + 2]>\n"; # ; should have either the end of line or a space or \ after it if ($op eq ';') { - if ($ctx !~ /.x[WE]/ && $cc !~ /^\\/) { - print "need space after that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + if ($ctx !~ /.x[WEB]/ && $cc !~ /^\\/ && + $cc !~ /^;/) { + ERROR("need space after that '$op' $at\n" . $hereptr); } # // is a comment @@ -662,43 +701,31 @@ sub process { # -> should have no spaces } elsif ($op eq '->') { if ($ctx =~ /Wx.|.xW/) { - print "no spaces around that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("no spaces around that '$op' $at\n" . $hereptr); } # , must have a space on the right. } elsif ($op eq ',') { if ($ctx !~ /.xW|.xE/ && $cc !~ /^}/) { - print "need space after that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("need space after that '$op' $at\n" . $hereptr); } # unary ! and unary ~ are allowed no space on the right } elsif ($op eq '!' or $op eq '~') { if ($ctx !~ /[WOEB]x./) { - print "need space before that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("need space before that '$op' $at\n" . $hereptr); } if ($ctx =~ /.xW/) { - print "no space after that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("no space after that '$op' $at\n" . $hereptr); } # unary ++ and unary -- are allowed no space on one side. } elsif ($op eq '++' or $op eq '--') { if ($ctx !~ /[WOB]x[^W]/ && $ctx !~ /[^W]x[WOBE]/) { - print "need space one side of that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("need space one side of that '$op' $at\n" . $hereptr); } if ($ctx =~ /Wx./ && $cc =~ /^;/) { - print "no space before that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("no space before that '$op' $at\n" . $hereptr); } # & is both unary and binary @@ -715,9 +742,7 @@ sub process { # } elsif ($op eq '&' or $op eq '-') { if ($ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]/) { - print "need space before that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("need space before that '$op' $at\n" . $hereptr); } # * is the same as & only adding: @@ -726,16 +751,9 @@ sub process { # (foo **) # } elsif ($op eq '*') { - if ($ca eq '*') { - if ($cc =~ /^\s(?!\s*const)/) { - print "no space after that '$op' $at\n"; - print "$hereptr"; - $clean = 0; - } - } elsif ($ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]|OxV|WxB|BxB/) { - print "need space before that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + if ($ca !~ /$Type$/ && $cb !~ /(\*$;|$;\*)/ && + $ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]|OxV|WxB|BxB/) { + ERROR("need space before that '$op' $at\n" . $hereptr); } # << and >> may either have or not have spaces both sides @@ -743,58 +761,51 @@ sub process { $op eq '^' or $op eq '|') { if ($ctx !~ /VxV|WxW|VxE|WxE/) { - print "need consistent spacing around '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("need consistent spacing around '$op' $at\n" . + $hereptr); } # All the others need spaces both sides. } elsif ($ctx !~ /[EW]x[WE]/) { - print "need spaces around that '$op' $at\n"; - print "$hereptr"; - $clean = 0; + ERROR("need spaces around that '$op' $at\n" . $hereptr); } $off += length($elements[$n + 1]); } } #need space before brace following if, while, etc - if ($line=~/\(.*\){/) { - print "need a space before the brace\n"; - print "$herecurr"; - $clean = 0; + if ($line =~ /\(.*\){/ || $line =~ /do{/) { + ERROR("need a space before the open brace '{'\n" . $herecurr); + } + +# closing brace should have a space following it when it has anything +# on the line + if ($line =~ /}(?!(?:,|;|\)))\S/) { + ERROR("need a space after that close brace '}'\n" . $herecurr); } #goto labels aren't indented, allow a single space however if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) { - print "labels should not be indented\n"; - print "$herecurr"; - $clean = 0; + WARN("labels should not be indented\n" . $herecurr); } # Need a space before open parenthesis after if, while etc if ($line=~/\b(if|while|for|switch)\(/) { - print "need a space before the open parenthesis\n"; - print "$herecurr"; - $clean = 0; + ERROR("need a space before the open parenthesis '('\n" . $herecurr); } # Check for illegal assignment in if conditional. if ($line=~/\bif\s*\(.*[^<>!=]=[^=].*\)/) { #next if ($line=~/\".*\Q$op\E.*\"/ or $line=~/\'\Q$op\E\'/); - print "do not use assignment in if condition\n"; - print "$herecurr"; - $clean = 0; + ERROR("do not use assignment in if condition\n" . $herecurr); } # Check for }else {, these must be at the same # indent level to be relevant to each other. if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and $previndent == $indent) { - print "else should follow close brace\n"; - print "$hereprev"; - $clean = 0; + ERROR("else should follow close brace '}'\n" . $hereprev); } #studly caps, commented out until figure out how to distinguish between use of existing and adding new @@ -806,57 +817,22 @@ sub process { #no spaces allowed after \ in define if ($line=~/\#define.*\\\s$/) { - print("Whitepspace after \\ makes next lines useless\n"); - print "$herecurr"; - $clean = 0; + WARN("Whitepspace after \\ makes next lines useless\n" . $herecurr); } #warn if is #included and is available (uses RAW line) if ($tree && $rawline =~ m{^.\#\s*include\s*\}) { my $checkfile = "include/linux/$1.h"; if (-f $checkfile) { - print "Use #include instead of \n"; - print $herecurr; - $clean = 0; - } - } - -# if/while/etc brace do not go on next line, unless defining a do while loop, -# or if that brace on the next line is for something else - if ($prevline=~/\b(?:(if|while|for|switch)\s*\(|do\b|else\b)/) { - my @opened = $prevline=~/\(/g; - my @closed = $prevline=~/\)/g; - my $nr_line = $linenr; - my $remaining = $realcnt - 1; - my $next_line = $line; - my $extra_lines = 0; - my $display_segment = $prevline; - - while ($remaining > 0 && scalar @opened > scalar @closed) { - $prevline .= $next_line; - $display_segment .= "\n" . $next_line; - $next_line = $lines[$nr_line]; - $nr_line++; - $remaining--; - - @opened = $prevline=~/\(/g; - @closed = $prevline=~/\)/g; - } - - if (($prevline=~/\b(?:(if|while|for|switch)\s*\(.*\)|do|else)\s*$/) and ($next_line=~/{/) and - !($next_line=~/\b(?:if|while|for|switch|do|else)\b/) and !($next_line=~/\#define.*do.*while/)) { - print "That { should be on the previous line\n"; - print "$here\n$display_segment\n$next_line\n\n"; - $clean = 0; + CHK("Use #include instead of \n" . + $herecurr); } } # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ && - $1 !~ /^\s*(?:\sif|{|$)/) { - print "trailing statements should be on next line\n"; - print "$herecurr"; - $clean = 0; + $1 !~ /^\s*(?:\sif|{|\\|$)/) { + ERROR("trailing statements should be on next line\n" . $herecurr); } # multi-statement macros should be enclosed in a do while loop, grab the @@ -877,49 +853,49 @@ sub process { $ln--; $cnt++; } - my $ctx = join('', ctx_statement($ln, $cnt)); + my @ctx = ctx_statement($ln, $cnt); + my $ctx_ln = $ln + $#ctx + 1; + my $ctx = join("\n", @ctx); + + # Pull in any empty extension lines. + while ($ctx =~ /\\$/ && + $lines[$ctx_ln - 1] =~ /^.\s*(?:\\)?$/) { + $ctx .= $lines[$ctx_ln - 1]; + $ctx_ln++; + } if ($ctx =~ /\\$/) { if ($ctx =~ /;/) { - print "Macros with multiple statements should be enclosed in a do - while loop\n"; + ERROR("Macros with multiple statements should be enclosed in a do - while loop\n" . "$here\n$ctx\n"); } else { - print "Macros with complex values should be enclosed in parenthesis\n"; + ERROR("Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n"); } - print "$hereprev"; - $clean = 0; } } # don't include deprecated include files (uses RAW line) for my $inc (@dep_includes) { if ($rawline =~ m@\#\s*include\s*\<$inc>@) { - print "Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n"; - print "$herecurr"; - $clean = 0; + ERROR("Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n" . $herecurr); } } # don't use deprecated functions for my $func (@dep_functions) { if ($line =~ /\b$func\b/) { - print "Don't use $func(): see Documentation/feature-removal-schedule.txt\n"; - print "$herecurr"; - $clean = 0; + ERROR("Don't use $func(): see Documentation/feature-removal-schedule.txt\n" . $herecurr); } } # no volatiles please if ($line =~ /\bvolatile\b/ && $line !~ /\basm\s+volatile\b/) { - print "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n"; - print "$herecurr"; - $clean = 0; + WARN("Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr); } # warn about #if 0 if ($line =~ /^.#\s*if\s+0\b/) { - print "#if 0 -- if this code redundant remove it\n"; - print "$herecurr"; - $clean = 0; + CHK("if this code is redundant consider removing it\n" . + $herecurr); } # warn about #ifdefs in C files @@ -933,41 +909,47 @@ sub process { if ($line =~ /^.\s*(struct\s+mutex|spinlock_t)\s+\S+;/) { my $which = $1; if (!ctx_has_comment($first_line, $linenr)) { - print "$1 definition without comment\n"; - print "$herecurr"; - $clean = 0; + CHK("$1 definition without comment\n" . $herecurr); } } # check for memory barriers without a comment. if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { if (!ctx_has_comment($first_line, $linenr)) { - print "memory barrier without comment\n"; - print "$herecurr"; - $clean = 0; + CHK("memory barrier without comment\n" . $herecurr); } } # check of hardware specific defines if ($line =~ m@^.#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@) { - print "architecture specific defines should be avoided\n"; - print "$herecurr"; - $clean = 0; + CHK("architecture specific defines should be avoided\n" . $herecurr); } +# check the location of the inline attribute, that it is between +# storage class and type. if ($line =~ /$Type\s+(?:inline|__always_inline)\b/ || $line =~ /\b(?:inline|always_inline)\s+$Storage/) { - print "inline keyword should sit between storage class and type\n"; - print "$herecurr"; - $clean = 0; + ERROR("inline keyword should sit between storage class and type\n" . $herecurr); + } + +# check for new externs in .c files. + if ($line =~ /^.\s*extern\s/ && ($realfile =~ /\.c$/)) { + WARN("externs should be avoided in .c files\n" . $herecurr); + } + +# checks for new __setup's + if ($rawline =~ /\b__setup\("([^"]*)"/) { + my $name = $1; + + if (!grep(/$name/, @setup_docs)) { + CHK("__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr); + } } } if ($chk_patch && !$is_patch) { - $clean = 0; - print "Does not appear to be a unified-diff format patch\n"; + ERROR("Does not appear to be a unified-diff format patch\n"); } if ($is_patch && $chk_signoff && $signoff == 0) { - $clean = 0; - print "Missing Signed-off-by: line(s)\n"; + ERROR("Missing Signed-off-by: line(s)\n"); } if ($clean == 1 && $quiet == 0) { -- cgit v1.2.3 From dcecc6c70013e3a5fa81b3081480c03e10670a23 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Sun, 15 Jul 2007 23:41:15 -0700 Subject: doc/oops-tracing: add Code: decode info Add info that the Code: bytes line contains or (wxyz) in some architecture oops reports and what that means. Add a script by Andi Kleen that reads the Code: line from an Oops report file and generates assembly code from the hex bytes. Signed-off-by: Randy Dunlap Cc: Andi Kleen Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- Documentation/oops-tracing.txt | 14 ++++++++++++ scripts/decodecode | 51 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 scripts/decodecode (limited to 'scripts') diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt index 7d5b60dea551..23e6dde7eea6 100644 --- a/Documentation/oops-tracing.txt +++ b/Documentation/oops-tracing.txt @@ -86,6 +86,20 @@ stuff are the values reported by the Oops - you can just cut-and-paste and do a replace of spaces to "\x" - that's what I do, as I'm too lazy to write a program to automate this all). +Alternatively, you can use the shell script in scripts/decodecode. +Its usage is: decodecode < oops.txt + +The hex bytes that follow "Code:" may (in some architectures) have a series +of bytes that precede the current instruction pointer as well as bytes at and +following the current instruction pointer. In some cases, one instruction +byte or word is surrounded by <> or (), as in "<86>" or "(f00d)". These +<> or () markings indicate the current instruction pointer. Example from +i386, split into multiple lines for readability: + +Code: f9 0f 8d f9 00 00 00 8d 42 0c e8 dd 26 11 c7 a1 60 ea 2b f9 8b 50 08 a1 +64 ea 2b f9 8d 34 82 8b 1e 85 db 74 6d 8b 15 60 ea 2b f9 <8b> 43 04 39 42 54 +7e 04 40 89 42 54 8b 43 04 3b 05 00 f6 52 c0 + Finally, if you want to see where the code comes from, you can do cd /usr/src/linux diff --git a/scripts/decodecode b/scripts/decodecode new file mode 100644 index 000000000000..1e1a8f620c47 --- /dev/null +++ b/scripts/decodecode @@ -0,0 +1,51 @@ +#!/bin/sh +# Disassemble the Code: line in Linux oopses +# usage: decodecode < oops.file +# +# options: set env. variable AFLAGS=options to pass options to "as"; +# e.g., to decode an i386 oops on an x86_64 system, use: +# AFLAGS=--32 decodecode < 386.oops + +T=`mktemp` +code= + +while read i ; do + +case "$i" in +*Code:*) + code=$i + ;; +esac + +done + +if [ -z "$code" ]; then + exit +fi + +echo $code +code=`echo $code | sed -e 's/.*Code: //'` + +marker=`expr index "$code" "\<"` +if [ $marker -eq 0 ]; then + marker=`expr index "$code" "\("` +fi + +if [ $marker -ne 0 ]; then + beforemark=`echo "$code" | cut -c-$((${marker} - 1))` + echo -n " .byte 0x" > $T.s + echo $beforemark | sed -e 's/ /,0x/g' >> $T.s + as $AFLAGS -o $T.o $T.s + objdump -S $T.o + rm $T.o $T.s + +# and fix code at-and-after marker + code=`echo "$code" | cut -c$((${marker} + 1))-` +fi + +code=`echo $code | sed -e 's/ [<(]/ /;s/[>)] / /;s/ /,0x/g'` +echo -n " .byte 0x" > $T.s +echo $code >> $T.s +as $AFLAGS -o $T.o $T.s +objdump -S $T.o +rm $T.o $T.s -- cgit v1.2.3 From 9281acea6a3687ff0f262e0be31eac34895b95d7 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Tue, 17 Jul 2007 04:03:51 -0700 Subject: kallsyms: make KSYM_NAME_LEN include space for trailing '\0' KSYM_NAME_LEN is peculiar in that it does not include the space for the trailing '\0', forcing all users to use KSYM_NAME_LEN + 1 when allocating buffer. This is nonsense and error-prone. Moreover, when the caller forgets that it's very likely to subtly bite back by corrupting the stack because the last position of the buffer is always cleared to zero. This patch increments KSYM_NAME_LEN by one and updates code accordingly. * off-by-one bug in asm-powerpc/kprobes.h::kprobe_lookup_name() macro is fixed. * Where MODULE_NAME_LEN and KSYM_NAME_LEN were used together, MODULE_NAME_LEN was treated as if it didn't include space for the trailing '\0'. Fix it. Signed-off-by: Tejun Heo Acked-by: Paulo Marques Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- arch/parisc/kernel/unwind.c | 2 +- fs/proc/base.c | 2 +- include/asm-powerpc/kprobes.h | 4 ++-- include/linux/kallsyms.h | 6 +++--- kernel/kallsyms.c | 16 ++++++++-------- kernel/lockdep.c | 4 ++-- kernel/module.c | 10 +++++----- kernel/time/timer_list.c | 2 +- kernel/time/timer_stats.c | 2 +- mm/slab.c | 2 +- scripts/kallsyms.c | 4 ++-- 11 files changed, 27 insertions(+), 27 deletions(-) (limited to 'scripts') diff --git a/arch/parisc/kernel/unwind.c b/arch/parisc/kernel/unwind.c index 322167737de7..cf780cb3b916 100644 --- a/arch/parisc/kernel/unwind.c +++ b/arch/parisc/kernel/unwind.c @@ -242,7 +242,7 @@ static void unwind_frame_regs(struct unwind_frame_info *info) #ifdef CONFIG_KALLSYMS /* Handle some frequent special cases.... */ { - char symname[KSYM_NAME_LEN+1]; + char symname[KSYM_NAME_LEN]; char *modname; kallsyms_lookup(info->ip, NULL, NULL, &modname, diff --git a/fs/proc/base.c b/fs/proc/base.c index ae3627337a92..42cb4f5613b6 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -283,7 +283,7 @@ static int proc_pid_auxv(struct task_struct *task, char *buffer) static int proc_pid_wchan(struct task_struct *task, char *buffer) { unsigned long wchan; - char symname[KSYM_NAME_LEN+1]; + char symname[KSYM_NAME_LEN]; wchan = get_wchan(task); diff --git a/include/asm-powerpc/kprobes.h b/include/asm-powerpc/kprobes.h index b0e40ff32ee0..9537fda238b8 100644 --- a/include/asm-powerpc/kprobes.h +++ b/include/asm-powerpc/kprobes.h @@ -65,10 +65,10 @@ typedef unsigned int kprobe_opcode_t; } else if (name[0] != '.') \ addr = *(kprobe_opcode_t **)addr; \ } else { \ - char dot_name[KSYM_NAME_LEN+1]; \ + char dot_name[KSYM_NAME_LEN]; \ dot_name[0] = '.'; \ dot_name[1] = '\0'; \ - strncat(dot_name, name, KSYM_NAME_LEN); \ + strncat(dot_name, name, KSYM_NAME_LEN - 2); \ addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name); \ } \ } diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h index 5f06527dca21..f73de6fb5c68 100644 --- a/include/linux/kallsyms.h +++ b/include/linux/kallsyms.h @@ -7,9 +7,9 @@ #include -#define KSYM_NAME_LEN 127 -#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \ - 2*(BITS_PER_LONG*3/10) + MODULE_NAME_LEN + 1) +#define KSYM_NAME_LEN 128 +#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + (KSYM_NAME_LEN - 1) + \ + 2*(BITS_PER_LONG*3/10) + (MODULE_NAME_LEN - 1) + 1) #ifdef CONFIG_KALLSYMS /* Lookup the address for a symbol. Returns 0 if not found. */ diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c index 0d662475dd9f..474219a41929 100644 --- a/kernel/kallsyms.c +++ b/kernel/kallsyms.c @@ -152,7 +152,7 @@ static unsigned int get_symbol_offset(unsigned long pos) /* Lookup the address for this symbol. Returns 0 if not found. */ unsigned long kallsyms_lookup_name(const char *name) { - char namebuf[KSYM_NAME_LEN+1]; + char namebuf[KSYM_NAME_LEN]; unsigned long i; unsigned int off; @@ -248,7 +248,7 @@ const char *kallsyms_lookup(unsigned long addr, { const char *msym; - namebuf[KSYM_NAME_LEN] = 0; + namebuf[KSYM_NAME_LEN - 1] = 0; namebuf[0] = 0; if (is_ksym_addr(addr)) { @@ -265,7 +265,7 @@ const char *kallsyms_lookup(unsigned long addr, /* see if it's in a module */ msym = module_address_lookup(addr, symbolsize, offset, modname); if (msym) - return strncpy(namebuf, msym, KSYM_NAME_LEN); + return strncpy(namebuf, msym, KSYM_NAME_LEN - 1); return NULL; } @@ -273,7 +273,7 @@ const char *kallsyms_lookup(unsigned long addr, int lookup_symbol_name(unsigned long addr, char *symname) { symname[0] = '\0'; - symname[KSYM_NAME_LEN] = '\0'; + symname[KSYM_NAME_LEN - 1] = '\0'; if (is_ksym_addr(addr)) { unsigned long pos; @@ -291,7 +291,7 @@ int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name) { name[0] = '\0'; - name[KSYM_NAME_LEN] = '\0'; + name[KSYM_NAME_LEN - 1] = '\0'; if (is_ksym_addr(addr)) { unsigned long pos; @@ -312,7 +312,7 @@ int sprint_symbol(char *buffer, unsigned long address) char *modname; const char *name; unsigned long offset, size; - char namebuf[KSYM_NAME_LEN+1]; + char namebuf[KSYM_NAME_LEN]; name = kallsyms_lookup(address, &size, &offset, &modname, namebuf); if (!name) @@ -342,8 +342,8 @@ struct kallsym_iter unsigned long value; unsigned int nameoff; /* If iterating in core kernel symbols */ char type; - char name[KSYM_NAME_LEN+1]; - char module_name[MODULE_NAME_LEN + 1]; + char name[KSYM_NAME_LEN]; + char module_name[MODULE_NAME_LEN]; int exported; }; diff --git a/kernel/lockdep.c b/kernel/lockdep.c index 1a5ff2211d88..edba2ffb43de 100644 --- a/kernel/lockdep.c +++ b/kernel/lockdep.c @@ -379,7 +379,7 @@ get_usage_chars(struct lock_class *class, char *c1, char *c2, char *c3, char *c4 static void print_lock_name(struct lock_class *class) { - char str[KSYM_NAME_LEN + 1], c1, c2, c3, c4; + char str[KSYM_NAME_LEN], c1, c2, c3, c4; const char *name; get_usage_chars(class, &c1, &c2, &c3, &c4); @@ -401,7 +401,7 @@ static void print_lock_name(struct lock_class *class) static void print_lockdep_cache(struct lockdep_map *lock) { const char *name; - char str[KSYM_NAME_LEN + 1]; + char str[KSYM_NAME_LEN]; name = lock->name; if (!name) diff --git a/kernel/module.c b/kernel/module.c index 539fed9ac83c..33c04ad51175 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2133,7 +2133,7 @@ int lookup_module_symbol_name(unsigned long addr, char *symname) sym = get_ksymbol(mod, addr, NULL, NULL); if (!sym) goto out; - strlcpy(symname, sym, KSYM_NAME_LEN + 1); + strlcpy(symname, sym, KSYM_NAME_LEN); mutex_unlock(&module_mutex); return 0; } @@ -2158,9 +2158,9 @@ int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, if (!sym) goto out; if (modname) - strlcpy(modname, mod->name, MODULE_NAME_LEN + 1); + strlcpy(modname, mod->name, MODULE_NAME_LEN); if (name) - strlcpy(name, sym, KSYM_NAME_LEN + 1); + strlcpy(name, sym, KSYM_NAME_LEN); mutex_unlock(&module_mutex); return 0; } @@ -2181,8 +2181,8 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type, *value = mod->symtab[symnum].st_value; *type = mod->symtab[symnum].st_info; strlcpy(name, mod->strtab + mod->symtab[symnum].st_name, - KSYM_NAME_LEN + 1); - strlcpy(module_name, mod->name, MODULE_NAME_LEN + 1); + KSYM_NAME_LEN); + strlcpy(module_name, mod->name, MODULE_NAME_LEN); *exported = is_exported(name, mod); mutex_unlock(&module_mutex); return 0; diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c index 8bbcfb77f7d2..e5edc3a22a08 100644 --- a/kernel/time/timer_list.c +++ b/kernel/time/timer_list.c @@ -38,7 +38,7 @@ DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases); static void print_name_offset(struct seq_file *m, void *sym) { - char symname[KSYM_NAME_LEN+1]; + char symname[KSYM_NAME_LEN]; if (lookup_symbol_name((unsigned long)sym, symname) < 0) SEQ_printf(m, "<%p>", sym); diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 9b8a826236dd..8ed62fda16c6 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -269,7 +269,7 @@ void timer_stats_update_stats(void *timer, pid_t pid, void *startf, static void print_name_offset(struct seq_file *m, unsigned long addr) { - char symname[KSYM_NAME_LEN+1]; + char symname[KSYM_NAME_LEN]; if (lookup_symbol_name(addr, symname) < 0) seq_printf(m, "<%p>", (void *)addr); diff --git a/mm/slab.c b/mm/slab.c index 35056394139b..96d30ee256ef 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -4344,7 +4344,7 @@ static void show_symbol(struct seq_file *m, unsigned long address) { #ifdef CONFIG_KALLSYMS unsigned long offset, size; - char modname[MODULE_NAME_LEN + 1], name[KSYM_NAME_LEN + 1]; + char modname[MODULE_NAME_LEN], name[KSYM_NAME_LEN]; if (lookup_symbol_attrs(address, &size, &offset, modname, name) == 0) { seq_printf(m, "%s+%#lx/%#lx", name, offset, size); diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c index 8b809b264d18..10b006694e5d 100644 --- a/scripts/kallsyms.c +++ b/scripts/kallsyms.c @@ -31,7 +31,7 @@ #include #include -#define KSYM_NAME_LEN 127 +#define KSYM_NAME_LEN 128 struct sym_entry { @@ -254,7 +254,7 @@ static void write_src(void) unsigned int i, k, off; unsigned int best_idx[256]; unsigned int *markers; - char buf[KSYM_NAME_LEN+1]; + char buf[KSYM_NAME_LEN]; printf("#include \n"); printf("#if BITS_PER_LONG == 64\n"); -- cgit v1.2.3