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

[Dwaipayan Ray]

On Sun, Oct 25, 2020 at 1:52 PM Joe Perches <joe@xxxxxxxxxxx> wrote:
>
> On Sun, 2020-10-25 at 12:21 +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.
> []
> > Also add fixes for the generic attributes check.
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > +                             if (exists($attr_list{$curr_attr})) {
> > +                                     my $new = $attr_list{$curr_attr};
> > +                                     if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> > +                                              "$new$params is preferred over __attribute__(($attr))\n" . $herecurr) &&
> > +                                             $fix) {
> > +                                             $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*\Q$attr\E\s*\)\s*\)/$new$params/;
>
> Thanks.
>
> This fix would only work for the single conversions
> and would not work for multiple attributes like:
>
>         __attribute__((aligned(4), packed))
>
> It would be nice to be able to convert this to
>
>         __aligned(4) __packed
>
> One mechanism to do that might be to:
>
>         create an empty array
>         for each attr
>                 push(@array, conversion)
>         s/__attribute__(...)/join(' ', @array)/
>
> if all attrs were converted.
>

Okay sure I will try doing that.

I think I need to also change the test to correctly display
the warning for those having multiple attributes too.
Because now only single attributes are reported.

And also do you think there should be a separate check
for __alias__(#symbol)? I think it expects a string and that should
be trimmed in the fix.

Thanks,
Dwaipayan.