Re: [PATCH v3] checkpatch: extend attributes check to handle more patterns

From: Joe Perches
Date: Sat Oct 24 2020 - 20:59:50 EST


On Sat, 2020-10-24 at 14:35 +0530, Dwaipayan Ray wrote:
> It is generally preferred that the macros from
> include/linux/compiler_attributes.h are used, unless there
> is a reason not to.
>
> checkpatch currently checks __attribute__ for each of
> packed, aligned, printf, scanf, and weak. Other declarations
> in compiler_attributes.h are not handled.
>
> Add a generic test to check the presence of such attributes.
> Some attributes require more specific handling and are kept
> separate.
[]
> - }
> + $line =~ /__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" => "__const",
> + "copy" => "__copy",
> + "designated_init" => "__designated_init",
> + "externally_visible" => "__visible",
> + "fallthrough" => "fallthrough",

I'd remove fallthrough.

It doesn't make sense as the attribute could be in any line
of a switch/case block and fallthrough; must be the last line
of the block.

> + if ($attr =~ /^(\w+)\s*(${balanced_parens})?/) {
> + my $curr_attr = $1;
> + my $params = '';
> + $params = $2 if defined($2);
> + $curr_attr =~ s/^[\s_]+|[\s_]+$//g;
> +
> + if (exists($attr_list{$curr_attr})) {
> + my $new = $attr_list{$curr_attr};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new$params is preffered over __attribute__(($attr))\n" . $herecurr);

Be nice to have a $fix option here

> + # Check for __attribute__ format(printf, prefer __printf
> + if ($attr =~ /^_*format_*\s*\(\s*printf/) {
> + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)\)\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex;

like for format(printf, index, pos)
and format(scanf, index, pos)