lint: checkpatch: Add SUSPICIOUS_CODE_INDENT test
This patch adds a new test to checkpatch that identifies cases where a line after a conditional statement is incorrectly intended (possibly indicating the mistake of forgetting to add braces), like this: if (a) b; c; Unfortunately, it seems like checkpatch is partially unmaintained in upstream Linux at the moment with maintainers either not responding at all or not even willing to look at new patches [1]. Since detecting this error class is important to coreboot, let's just carry this feature locally for now. [1] https://lkml.org/lkml/2021/4/15/1488 Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I7bb90b56dfc7582271d2b82cb42a2c1df477054f Reviewed-on: https://review.coreboot.org/c/coreboot/+/51838 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Nico Huber <nico.h@gmx.de>
This commit is contained in:
parent
aee70a8a8e
commit
218906df66
|
@ -1197,8 +1197,12 @@ sub copy_spacing {
|
||||||
sub line_stats {
|
sub line_stats {
|
||||||
my ($line) = @_;
|
my ($line) = @_;
|
||||||
|
|
||||||
# Drop the diff line leader and expand tabs
|
# Drop the diff line leader
|
||||||
$line =~ s/^.//;
|
$line =~ s/^.//;
|
||||||
|
|
||||||
|
# Treat labels like whitespace when counting indentation
|
||||||
|
$line =~ s/^( ?$Ident:)/" " x length($1)/e;
|
||||||
|
|
||||||
$line = expand_tabs($line);
|
$line = expand_tabs($line);
|
||||||
|
|
||||||
# Pick the indent from the front of the line.
|
# Pick the indent from the front of the line.
|
||||||
|
@ -1331,15 +1335,13 @@ sub ctx_statement_block {
|
||||||
|
|
||||||
my $type = '';
|
my $type = '';
|
||||||
my $level = 0;
|
my $level = 0;
|
||||||
my @stack = ();
|
my @stack = (['', $level]);
|
||||||
my $p;
|
my $p;
|
||||||
my $c;
|
my $c;
|
||||||
my $len = 0;
|
my $len = 0;
|
||||||
|
|
||||||
my $remainder;
|
my $remainder;
|
||||||
while (1) {
|
while (1) {
|
||||||
@stack = (['', 0]) if ($#stack == -1);
|
|
||||||
|
|
||||||
#warn "CSB: blk<$blk> remain<$remain>\n";
|
#warn "CSB: blk<$blk> remain<$remain>\n";
|
||||||
# If we are about to drop off the end, pull in more
|
# If we are about to drop off the end, pull in more
|
||||||
# context.
|
# context.
|
||||||
|
@ -1373,9 +1375,9 @@ sub ctx_statement_block {
|
||||||
# Handle nested #if/#else.
|
# Handle nested #if/#else.
|
||||||
if ($remainder =~ /^#\s*(?:ifndef|ifdef|if)\s/) {
|
if ($remainder =~ /^#\s*(?:ifndef|ifdef|if)\s/) {
|
||||||
push(@stack, [ $type, $level ]);
|
push(@stack, [ $type, $level ]);
|
||||||
} elsif ($remainder =~ /^#\s*(?:else|elif)\b/) {
|
} elsif ($remainder =~ /^#\s*(?:else|elif)\b/ && $#stack > 0) {
|
||||||
($type, $level) = @{$stack[$#stack - 1]};
|
($type, $level) = @{$stack[$#stack]};
|
||||||
} elsif ($remainder =~ /^#\s*endif\b/) {
|
} elsif ($remainder =~ /^#\s*endif\b/ && $#stack > 0) {
|
||||||
($type, $level) = @{pop(@stack)};
|
($type, $level) = @{pop(@stack)};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1545,9 +1547,9 @@ sub ctx_block_get {
|
||||||
# Handle nested #if/#else.
|
# Handle nested #if/#else.
|
||||||
if ($lines[$line] =~ /^.\s*#\s*(?:ifndef|ifdef|if)\s/) {
|
if ($lines[$line] =~ /^.\s*#\s*(?:ifndef|ifdef|if)\s/) {
|
||||||
push(@stack, $level);
|
push(@stack, $level);
|
||||||
} elsif ($lines[$line] =~ /^.\s*#\s*(?:else|elif)\b/) {
|
} elsif ($lines[$line] =~ /^.\s*#\s*(?:else|elif)\b/ && $#stack > 0) {
|
||||||
$level = $stack[$#stack - 1];
|
$level = $stack[$#stack];
|
||||||
} elsif ($lines[$line] =~ /^.\s*#\s*endif\b/) {
|
} elsif ($lines[$line] =~ /^.\s*#\s*endif\b/ && $#stack > 0) {
|
||||||
$level = pop(@stack);
|
$level = pop(@stack);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -3615,6 +3617,50 @@ sub process {
|
||||||
WARN("SUSPECT_CODE_INDENT",
|
WARN("SUSPECT_CODE_INDENT",
|
||||||
"suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n");
|
"suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Also check if the next statement after the previous condition has the same indent
|
||||||
|
my ($stat_next, undef, $line_nr_next_next) =
|
||||||
|
ctx_statement_block($line_nr_next, $remain_next, $off_next);
|
||||||
|
my $s_next = $stat_next;
|
||||||
|
|
||||||
|
# Remove line prefixes
|
||||||
|
$s_next =~ s/\n./\n/g;
|
||||||
|
|
||||||
|
# Remove any comments
|
||||||
|
$s_next =~ s/$;//g;
|
||||||
|
|
||||||
|
# Remove any leading labels
|
||||||
|
$s_next =~ s/\n( ?$Ident:)/"\n" . " " x length($1)/eg;
|
||||||
|
|
||||||
|
# Skip this check for in case next statement starts with 'else'
|
||||||
|
if ($s_next !~ /\belse\b/) {
|
||||||
|
|
||||||
|
# Remove while that belongs to a do {} while
|
||||||
|
if ($stat =~ /\bdo\b/) {
|
||||||
|
$s_next =~ s/^.*\bwhile\b\s*($balanced_parens)\s*?//;
|
||||||
|
}
|
||||||
|
|
||||||
|
# Remove blank lines
|
||||||
|
$s_next =~ s/\s*\\?\n//g;
|
||||||
|
|
||||||
|
# Get the real next lines
|
||||||
|
my $next_nof_lines = $line_nr_next_next - $line_nr_next;
|
||||||
|
my $stat_next_real = raw_line($line_nr_next, $next_nof_lines);
|
||||||
|
if (!defined($stat_next_real)) {
|
||||||
|
$stat_next_real = "";
|
||||||
|
} elsif ($next_nof_lines > 1) {
|
||||||
|
$stat_next_real = "[...]\n$stat_next_real";
|
||||||
|
}
|
||||||
|
my (undef, $nindent) = line_stats('+' . $s_next);
|
||||||
|
|
||||||
|
#print "stat_next<$stat_next> stat<$stat> indent<$indent> nindent<$nindent> s_next<$s_next> stat_next_real<$stat_next_real>\n";
|
||||||
|
|
||||||
|
if ($nindent > $indent) {
|
||||||
|
WARN("SUSPICIOUS_CODE_INDENT",
|
||||||
|
"suspicious code indentation after conditional statements\n" .
|
||||||
|
$herecurr . "$stat_real\n$stat_next_real\n");
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
# Track the 'values' across context and added lines.
|
# Track the 'values' across context and added lines.
|
||||||
|
|
Loading…
Reference in New Issue