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

[Dwaipayan Ray]

On Fri, Oct 23, 2020 at 4:34 PM Joe Perches <joe@xxxxxxxxxxx> wrote:
>
> 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))) {
>

Okay thanks!
But what should be done for the attributes which are
parameterized, like __aligned__(x). Should all the __
for these entries be trimmed too? There are also
cases where there are multiple versions like:

__aligned__
__aligned__(x)

To help differentiate between them what can be done?
Should i make the keys as:

aligned
aligned__(

instead of

__aligned__
__aligned__(

Thanks,
Dwaipayan.