lint/kconfig: More checks, more errors

This patch changes a few more Kconfig linter warnings to errors that
currently do not show up in the tree and that seem unlikely to become
false positive in the future. One instance of duplicated code that
essentially checks for the same thing was consolidated.

It also adds a new test for references to boolean Kconfig options that
do not use the CONFIG() wrapper macro. It's a little flaky (e.g. hard to
handle multi-line comments), but it should be helpful the majority of
the time as a warning in a Jenkins comment.

Change-Id: I975ee77d392ed426f76f7671d9b6ef9441656e6a
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31777
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
This commit is contained in:
Julius Werner 2019-03-05 17:10:19 -08:00 committed by Patrick Georgi
parent b08906b240
commit e5eb2decd0
2 changed files with 39 additions and 38 deletions

View File

@ -199,35 +199,16 @@ sub check_for_ifdef {
my $symbol = $3; my $symbol = $3;
if ( ( exists $symbols{$symbol} ) && ( $symbols{$symbol}{type} ne "string" ) ) { if ( ( exists $symbols{$symbol} ) && ( $symbols{$symbol}{type} ne "string" ) ) {
show_warning( "#ifdef 'CONFIG_$symbol' used at $file:$lineno." show_error( "#ifdef 'CONFIG_$symbol' used at $file:$lineno."
. " Symbols of type '$symbols{$symbol}{type}' are always defined." ); . " Symbols of type '$symbols{$symbol}{type}' are always defined." );
} }
} elsif ( $line =~ /^([^:]+):(\d+):\s*#\s*if\s+!?\s*defined\s*\(?\s*CONFIG(?:_|\()(\w+)/ ) { } elsif ( $line =~ /^([^:]+):(\d+):.+defined\s*\(?\s*CONFIG(?:_|\()(\w+)/ ) {
my $file = $1; my $file = $1;
my $lineno = $2; my $lineno = $2;
my $symbol = $3; my $symbol = $3;
if ( ( exists $symbols{$symbol} ) && ( $symbols{$symbol}{type} ne "string" ) ) { if ( ( exists $symbols{$symbol} ) && ( $symbols{$symbol}{type} ne "string" ) ) {
show_warning( "#ifdef 'CONFIG_$symbol' used at $file:$lineno." show_error( "defined(CONFIG_$symbol) used at $file:$lineno."
. " Symbols of type '$symbols{$symbol}{type}' are always defined." );
}
}
}
# look for (#if) defined SYMBOL
@ifdef_symbols = @collected_symbols;
while ( my $line = shift @ifdef_symbols ) {
if ( $line =~ /^([^:]+):(\d+):.+defined\s*\(\s*CONFIG(?:_|\()(\w+)/ ) {
my $file = $1;
my $lineno = $2;
my $symbol = $3;
#ignore '#if defined(symbol) && symbol' type statements
next
if ( $line =~ /^([^:]+):(\d+):.+defined\s*\(\s*CONFIG_$symbol.*(&&|\|\|)\s*!?\s*\(?\s*CONFIG_$symbol/ );
if ( ( exists $symbols{$symbol} ) && ( $symbols{$symbol}{type} ne "string" ) ) {
show_warning( "defined 'CONFIG_$symbol' used at $file:$lineno."
. " Symbols of type '$symbols{$symbol}{type}' are always defined." ); . " Symbols of type '$symbols{$symbol}{type}' are always defined." );
} }
} }
@ -272,10 +253,10 @@ sub check_for_def {
my $symbol = $3; my $symbol = $3;
if ( ( exists $symbols{$symbol} ) ) { if ( ( exists $symbols{$symbol} ) ) {
show_warning("#define of symbol 'CONFIG_$symbol' used at $file:$lineno."); show_error("#define of symbol 'CONFIG_$symbol' used at $file:$lineno.");
} }
else { else {
show_warning( "#define 'CONFIG_$symbol' used at $file:$lineno." show_error( "#define 'CONFIG_$symbol' used at $file:$lineno."
. " Other #defines should not look like Kconfig symbols." ); . " Other #defines should not look like Kconfig symbols." );
} }
} }
@ -383,6 +364,24 @@ sub check_is_enabled {
. " CONFIG($symbol) should be used for type 'bool'" ); . " CONFIG($symbol) should be used for type 'bool'" );
} }
} }
} elsif ( $line =~ /^([^:]+):(\d+):(.+\bCONFIG_.+)/ ) {
my $file = $1;
my $lineno = $2;
$line = $3;
if ( $file =~ /.*\.(c|h|asl|ld)/ ) {
while ( $line =~ /(.*)\bCONFIG_(\w+)(.*)/ && $1 !~ /\/\/|\/\*/ ) {
my $symbol = $2;
$line = $1 . $3;
if ( exists $symbols{$symbol} ) {
if ( $symbols{$symbol}{type} eq "bool" ) {
show_warning( "Naked reference to CONFIG_$symbol used at $file:$lineno."
. " A 'bool' Kconfig should always be accessed through CONFIG($symbol)." );
}
} else {
show_warning( "Unknown config option CONFIG_$symbol used at $file:$lineno." );
}
}
}
} }
} }
} }
@ -437,7 +436,7 @@ sub check_defaults {
} }
} elsif ($symbols{$sym}{type} eq "bool") { } elsif ($symbols{$sym}{type} eq "bool") {
if ($symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /[01YN]/) { if ($symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /[01YN]/) {
show_warning("default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) for bool symbol $sym uses value other than y/n at $filename:$line_no."); show_error("default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) for bool symbol $sym uses value other than y/n at $filename:$line_no.");
} else { } else {
show_error("non bool default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) used for bool symbol $sym at $filename:$line_no."); show_error("non bool default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) used for bool symbol $sym at $filename:$line_no.");
} }
@ -447,7 +446,7 @@ sub check_defaults {
#if a default is already set, display an error #if a default is already set, display an error
if ($default_set) { if ($default_set) {
show_warning( "Default for '$sym' referenced at $filename:$line_no will never be set" show_error( "Default for '$sym' referenced at $filename:$line_no will never be set"
. " - overridden by default set at $default_filename:$default_line_no" ); . " - overridden by default set at $default_filename:$default_line_no" );
} }
else { else {
@ -1140,7 +1139,7 @@ sub handle_type {
( $type, $expression ) = handle_if_line( $type, $inside_config, $filename, $line_no ); ( $type, $expression ) = handle_if_line( $type, $inside_config, $filename, $line_no );
if ( $type =~ /tristate/ ) { if ( $type =~ /tristate/ ) {
show_warning("$filename:$line_no - tristate types are not used."); show_error("$filename:$line_no - tristate types are not used.");
} }
if ($inside_config) { if ($inside_config) {
@ -1265,7 +1264,7 @@ sub load_kconfig_file {
#the directory should exist when using a glob #the directory should exist when using a glob
else { else {
show_warning("Could not find dir '$dir_prefix'"); show_error("Could not find dir '$dir_prefix'");
} }
} }

View File

@ -43,23 +43,20 @@ Notes:
- Show when the range set for a hex or int does not match a previous range - Show when the range set for a hex or int does not match a previous range
Warnings in Kconfig files: Warnings in Kconfig files:
- Any 'default' expressions that can never be reached.
- Symbols that are defined but never used. - Symbols that are defined but never used.
- Directories specified in a 'source' keyword do not exist.
- A 'source' keyword loading a Kconfig file that has already been loaded. - A 'source' keyword loading a Kconfig file that has already been loaded.
- A 'source' keyword loading a Kconfig file that doesn't exist. Note that - A 'source' keyword loading a Kconfig file that doesn't exist. Note that
globs are excluded from this check. globs are excluded from this check.
Warnings in coreboot source files: Warnings in coreboot source files:
- #define of Kconfig symbol - Symbols should only be defined in Kconfig.
- #define starting with 'CONFIG_' - these should be reserved for Kconfig
symbols.
- 'IS_ENABLED()' block that could not be interpreted. - 'IS_ENABLED()' block that could not be interpreted.
- Kconfig files that are not loaded by a 'source' keyword. - Kconfig files that are not loaded by a 'source' keyword.
- '#ifdef' or '#if defined' used on bool, int, or hex - these are always - IS_ENABLED() used on a Kconfig (deprecated in favor of CONFIG())
defined in coreboot's version of Kconfig. - Naked use of boolean CONFIG_XXX Kconfig in C that's not wrapped in CONFIG()
Errors in Kconfig files: Errors in Kconfig files:
- Any 'default' expressions that can never be reached.
- Directories specified in a 'source' keyword do not exist.
- Selects do not work on symbols created in a choice block. - Selects do not work on symbols created in a choice block.
- All symbols used in selects or expressions must be defined in a config - All symbols used in selects or expressions must be defined in a config
statement. statement.
@ -94,8 +91,13 @@ Errors in Kconfig that are also caught by Kconfig itself:
- Symbols with no defined type. - Symbols with no defined type.
Errors in coreboot source files: Errors in coreboot source files:
- The IS_ENABLED macro is only valid for bool symbols. - #define of Kconfig symbol - Symbols should only be defined in Kconfig.
- The IS_ENABLED used on unknown CONFIG_ value, like an obsolete symbol. - #define starting with 'CONFIG_' - these should be reserved for Kconfig
- The IS_ENABLED macro is used on a symbol without the CONFIG_ prefix. symbols.
- '#ifdef' or '#if defined' used on bool, int, or hex - these are always
defined in coreboot's version of Kconfig.
- The CONFIG() and IS_ENABLED() macros is only valid for bool symbols.
- CONFIG() or IS_ENABLED() used on unknown Kconfig, like an obsolete symbol.
- The IS_ENABLED() macro is used on a symbol without the CONFIG_ prefix.
TODO: check for choice entries at the top level TODO: check for choice entries at the top level