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)


Reply via email to