Skip to content

Commit

Permalink
scripts/checkpatch.pl: some improvements to reduce false positives
Browse files Browse the repository at this point in the history
- camelcase is OK for printk inttypes.h;
- strncpy is OK;
- accept up to 120 chars on lines without warnings;
- stop complaining about "BACKTRACE=" strings split on multiple lines;
- remove PREFER_DEFINED_ATTRIBUTE_MACRO, as this is kernel-specific;
- remove MACRO_ARG_REUSE, as this applies mostly to multithreading;
- don't warn on using do{} while(0) with single line statements;

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
  • Loading branch information
mchehab committed Jul 17, 2024
1 parent 7a69b22 commit bb513b7
Showing 1 changed file with 7 additions and 96 deletions.
103 changes: 7 additions & 96 deletions scripts/checkpatch.pl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
my @ignore = ();
my $help = 0;
my $configuration_file = ".checkpatch.conf";
my $max_line_length = 100;
my $max_line_length = 120;
my $ignore_perl_version = 0;
my $minimum_perl_version = 5.10.0;
my $min_conf_desc_length = 4;
Expand Down Expand Up @@ -5636,12 +5636,10 @@ sub process {
#CamelCase
if ($var !~ /^$Constant$/ &&
$var =~ /[A-Z][a-z]|[a-z][A-Z]/ &&
#Ignore inttypes.h macros
$var !~ /^_*PRI(b|d|i|o|u|x)(8|16|32|64|PTR)/ &&
#Ignore some autogenerated defines and enum values
$var !~ /^(?:[A-Z]+_){1,5}[A-Z]{1,3}[a-z]/ &&
#Ignore Page<foo> variants
$var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ &&
#Ignore ETHTOOL_LINK_MODE_<foo> variants
$var !~ /^ETHTOOL_LINK_MODE_/ &&
#Ignore SI style variants like nS, mV and dB
#(ie: max_uV, regulator_min_uA_show, RANGE_mA_VALUE)
$var !~ /^(?:[a-z0-9_]*|[A-Z0-9_]*)?_?[a-z][A-Z](?:_[a-z0-9_]+|_[A-Z0-9_]+)?$/ &&
Expand Down Expand Up @@ -5825,10 +5823,6 @@ sub process {
$tmp_stmt =~ s/\#+\s*$arg\b//g;
$tmp_stmt =~ s/\b$arg\s*\#\#//g;
my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g;
if ($use_cnt > 1) {
CHK("MACRO_ARG_REUSE",
"Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx");
}
# check if any macro arguments may have other precedence issues
if ($tmp_stmt =~ m/($Operators)?\s*\b$arg\b\s*($Operators)?/m &&
((defined($1) && $1 ne ',') ||
Expand Down Expand Up @@ -5894,11 +5888,6 @@ sub process {
my $cnt = statement_rawlines($ctx);
my $herectx = get_stat_here($linenr, $cnt, $here);

if (($stmts =~ tr/;/;/) == 1 &&
$stmts !~ /^\s*(if|while|for|switch)\b/) {
WARN("SINGLE_STATEMENT_DO_WHILE_MACRO",
"Single statement macros should not use a do {} while (0) loop\n" . "$herectx");
}
if (defined $semis && $semis ne "") {
WARN("DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON",
"do {} while (0) macros should not be semicolon terminated\n" . "$herectx");
Expand Down Expand Up @@ -6058,11 +6047,10 @@ sub process {

# Check for user-visible strings broken across lines, which breaks the ability
# to grep for the string. Make exceptions when the previous string ends in a
# newline (multiple lines in one string constant) or '\t', '\r', ';', or '{'
# (common in inline assembly) or is a octal \123 or hexadecimal \xaf value
# newline (multiple lines in one string constant) or '\t', '\r' or '='
if ($line =~ /^\+\s*$String/ &&
$prevline =~ /"\s*$/ &&
$prevrawline !~ /(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) {
$prevrawline !~ /(?:\\(?:[ntr])|=)"\s*$/) {
if (WARN("SPLIT_STRING",
"quoted string split across lines\n" . $hereprev) &&
$fix &&
Expand Down Expand Up @@ -6508,71 +6496,6 @@ sub process {
}
}

# Check for compiler attributes
if ($realfile !~ m@\binclude/uapi/@ &&
$rawline =~ /\b__attribute__\s*\(\s*($balanced_parens)\s*\)/) {
my $attr = $1;
$attr =~ s/\s*\(\s*(.*)\)\s*/$1/;

my %attr_list = (
"alias" => "__alias",
"aligned" => "__aligned",
"always_inline" => "__always_inline",
"assume_aligned" => "__assume_aligned",
"cold" => "__cold",
"const" => "__attribute_const__",
"copy" => "__copy",
"designated_init" => "__designated_init",
"externally_visible" => "__visible",
"format" => "printf|scanf",
"gnu_inline" => "__gnu_inline",
"malloc" => "__malloc",
"mode" => "__mode",
"no_caller_saved_registers" => "__no_caller_saved_registers",
"noclone" => "__noclone",
"noinline" => "noinline",
"nonstring" => "__nonstring",
"noreturn" => "__noreturn",
"packed" => "__packed",
"pure" => "__pure",
"section" => "__section",
"used" => "__used",
"weak" => "__weak"
);

while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) {
my $orig_attr = $1;
my $params = '';
$params = $2 if defined($2);
my $curr_attr = $orig_attr;
$curr_attr =~ s/^[\s_]+|[\s_]+$//g;
if (exists($attr_list{$curr_attr})) {
my $new = $attr_list{$curr_attr};
if ($curr_attr eq "format" && $params) {
$params =~ /^\s*\(\s*(\w+)\s*,\s*(.*)/;
$new = "__$1\($2";
} else {
$new = "$new$params";
}
if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
"Prefer $new over __attribute__(($orig_attr$params))\n" . $herecurr) &&
$fix) {
my $remove = "\Q$orig_attr\E" . '\s*' . "\Q$params\E" . '(?:\s*,\s*)?';
$fixed[$fixlinenr] =~ s/$remove//;
$fixed[$fixlinenr] =~ s/\b__attribute__/$new __attribute__/;
$fixed[$fixlinenr] =~ s/\}\Q$new\E/} $new/;
$fixed[$fixlinenr] =~ s/ __attribute__\s*\(\s*\(\s*\)\s*\)//;
}
}
}

# Check for __attribute__ unused, prefer __always_unused or __maybe_unused
if ($attr =~ /^_*unused/) {
WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
"__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr);
}
}

# Check for __attribute__ weak, or __weak declarations (may have link issues)
if ($perl_version_ok &&
$line =~ /(?:$Declare|$DeclareMisordered)\s*$Ident\s*$balanced_parens\s*(?:$Attribute)?\s*;/ &&
Expand Down Expand Up @@ -6752,22 +6675,10 @@ sub process {
# }
# }

# strcpy uses that should likely be strscpy
# strcpy should be avoided
if ($line =~ /\bstrcpy\s*\(/) {
WARN("STRCPY",
"Prefer strscpy over strcpy - see: https://github.com/KSPP/linux/issues/88\n" . $herecurr);
}

# strlcpy uses that should likely be strscpy
if ($line =~ /\bstrlcpy\s*\(/) {
WARN("STRLCPY",
"Prefer strscpy over strlcpy - see: https://github.com/KSPP/linux/issues/89\n" . $herecurr);
}

# strncpy uses that should likely be strscpy or strscpy_pad
if ($line =~ /\bstrncpy\s*\(/) {
WARN("STRNCPY",
"Prefer strscpy, strscpy_pad, or __nonstring over strncpy - see: https://github.com/KSPP/linux/issues/90\n" . $herecurr);
"Please avoid strcpy\n" . $herecurr);
}

# ethtool_sprintf uses that should likely be ethtool_puts
Expand Down

0 comments on commit bb513b7

Please sign in to comment.