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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABJPP5D9ziuyPpPPyPM+n101vT1LGEjHj+JQjOhdN533t8Ud7w@mail.gmail.com>
Date:   Sun, 25 Oct 2020 14:02:32 +0530
From:   Dwaipayan Ray <dwaipayanray1@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH v4] checkpatch: extend attributes check to handle more patterns

On Sun, Oct 25, 2020 at 1:52 PM Joe Perches <joe@...ches.com> 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.