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

[Joe Perches <joe@perches.com>]

On Fri, 2020-10-23 at 15:13 +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

checkpatch, no need for capitalization

and non-trivially:

> +			my $attr_list = qr {
> +				__alias__|
> +				__aligned__$|
> +				__aligned__\(.*\)|
> +				__always_inline__|
> +				__assume_aligned__\(.*\)|
> +				__cold__|
> +				__const__|
> +				__copy__\(.*\)|
> +				__designated_init__|
> +				__externally_visible__|
> +				__fallthrough__|
> +				__gnu_inline__|
> +				__malloc__|
> +				__mode__\(.*\)|
> +				__no_caller_saved_registers__|
> +				__noclone__|
> +				__noinline__|
> +				__nonstring__|
> +				__noreturn__|
> +				__packed__|
> +				__pure__|
> +				__used__
> +			}x;
[]
> +			my %attr_replace = (
> +				"__alias__"			=> "__alias",
> +				"__aligned__"		=> "__aligned_largest",
> +				"__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",
> +				"__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",
> +				"__used__"			=> "__used"
> +			);
> +
> +			if ($attr =~/^($attr_list)/) {

I would remove the __ from the entries here.

And you could check using

	$line =~ /__attribute__\s*\(\s*($balanced_parens)\s*)/

and check for all attributes in $1 after
stripping the leading and trailing parens
and any leading and trailing underscores
from each attribute.

And you only need one hash and you should 
check for existence of the key rather than
have multiple lists.

	if (exists($attributes($attr))) {

> +				my $old = $1;
> +				if ($old =~ /^(__.+__\()(.*)\)/) {
> +					my $new = $attr_replace{$1};
> +					WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> +					     "$new($2) is preffered over __attribute__(($old))\n" . $herecurr);

preferred

> +				} else {
> +					my $new = $attr_replace{$old};
> +					WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> +					     "$new is preffered over __attribute__(($old))\n" . $herecurr);
> +				}
> +			}