From: Dwaipayan Ray <dwaipayanray1@gmail.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH RFC] checkpatch: extend attributes check to handle more patterns
Date: Thu, 22 Oct 2020 18:27:39 +0530 [thread overview]
Message-ID: <CABJPP5AObU2e2u8Zcd1J7k80dSqGPYGXQ_0RQVduBogzg6XJSg@mail.gmail.com> (raw)
In-Reply-To: <20201022124741.620334-1-dwaipayanray1@gmail.com>
On Thu, Oct 22, 2020 at 6:18 PM Dwaipayan Ray <dwaipayanray1@gmail.com> 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 more definitions to the attribute check.
> The following patterns are added:
>
> __alias__(#symbol)
> __always_inline__
> __assume_aligned__(a, ## __VA_ARGS__)
> __cold__
> __const__
> __copy__(symbol)
> __designated_init__
> __externally_visible__
> __gnu_inline__
> __malloc__
> __mode__(x)
> __no_caller_saved_registers__
> __noclone__
> __fallthrough__
> __noinline__
> __nonstring__
> __noreturn__
> __pure__
> __unused__
> __used__
>
> Link: https://lore.kernel.org/linux-kernel-mentees/3ec15b41754b01666d94b76ce51b9832c2dd577a.camel@perches.com/
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> ---
> scripts/checkpatch.pl | 199 ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 163 insertions(+), 36 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7e505688257a..869d9b2801cd 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6155,50 +6155,177 @@ sub process {
> }
> }
>
> -# Check for __attribute__ packed, prefer __packed
> +# Check for compiler attributes
> if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(.*\bpacked\b/) {
> - WARN("PREFER_PACKED",
> - "__packed is preferred over __attribute__((packed))\n" . $herecurr);
> - }
> + $line =~ /\b__attribute__\s*\(\s*\((.*)\)\s*\)/) {
> + my $attr = trim($1);
>
> -# Check for __attribute__ aligned, prefer __aligned
> - if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(.*aligned/) {
> - WARN("PREFER_ALIGNED",
> - "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr);
> - }
> + # Check for __attribute__ alias, prefer __alias
> + if ($attr =~/^_*alias_*/) {
> + WARN("PREFER_ALIAS",
> + "__alias(symbol) is preferred over __attribute__((alias(#symbol)))\n" . $herecurr);
> + }
>
> -# Check for __attribute__ section, prefer __section
> - if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) {
> - my $old = substr($rawline, $-[1], $+[1] - $-[1]);
> - my $new = substr($old, 1, -1);
> - if (WARN("PREFER_SECTION",
> - "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr) &&
> - $fix) {
> - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*\)\s*\)\s*\)/__section($new)/;
> + # Check for __attribute__ aligned(size), prefer __aligned(size)
> + if ($attr =~/^_*aligned_*\(/) {
> + WARN("PREFER_ALIGNED",
> + "__aligned(size) is preferred over __attribute__((__aligned__(size)))\n" . $herecurr);
> }
> - }
>
> -# Check for __attribute__ format(printf, prefer __printf
> - if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf/) {
> - if (WARN("PREFER_PRINTF",
> - "__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;
> + # Check for __attribute__ aligned, prefer __aligned_largest
> + if ($attr =~/^_*aligned_*$/) {
> + WARN("PREFER_ALIGNED_LARGEST",
> + "__aligned_largest is preferred over __attribute__((__aligned__))\n" . $herecurr);
> + }
>
> + # Check for __attribute__ always_inline, prefer __always_inline
> + if ($attr =~/^_*always_inline_*/) {
> + WARN("PREFER_ALWAYS_INLINE",
> + "__always_inline is preferred over __attribute__((__always_inline__))\n" . $herecurr);
> }
> - }
>
> -# Check for __attribute__ format(scanf, prefer __scanf
> - if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\b/) {
> - if (WARN("PREFER_SCANF",
> - "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) &&
> - $fix) {
> - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex;
> + # Check for __attribute__ assume_aligned, prefer __assume_aligned
> + if ($attr =~/^_*assume_aligned_*/) {
> + WARN("PREFER_ASSUME_ALIGNED",
> + "__asume_aligned(a, ...) is preferred over __attribute__((__assume_aligned__(a, ## __VA_ARGS__)))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ cold, prefer __cold
> + if ($attr =~ /^_*cold_*/) {
> + WARN("PREFER_COLD",
> + "__cold is preferred over __attribute__((__cold__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ const, prefer __attribute_const
> + if ($attr =~ /^_*const_*/) {
> + WARN("PREFER_CONST",
> + "__attribute_const is preferred over __attribute__((__const__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ copy, prefer __copy
> + if ($attr =~ /^_*copy_*/) {
> + WARN("PREFER_COPY",
> + "__copy(symbol) is preferred over __attribute__((__copy__(symbol)))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ desginated_init, prefer __designated_init
> + if ($attr =~ /^_*designated_init_*/) {
> + WARN("PREFER_DESIGNATED_INIT",
> + "__designated_init is preferred over __attribute__((__designated_init__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ externally_visible, prefer __visible
> + if ($attr =~ /^_*externally_visible_*/) {
> + WARN("PREFER_VISIBLE",
> + "__visible is preferred over __attribute__((__externally_visible__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ format(printf, prefer __printf
> + if ($attr =~ /^_*format_*\s*\(\s*printf/) {
> + if (WARN("PREFER_PRINTF",
> + "__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;
> +
> + }
> + }
> +
> + # Check for __attribute__ format(scanf, prefer __scanf
> + if ($attr =~ /^_*format_*\s*\(\s*scanf\b/) {
> + if (WARN("PREFER_SCANF",
> + "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex;
> + }
> + }
> +
> + # Check for __attribute__ gnu_inline, prefer __gnu_inline
> + if ($attr =~ /^_*gnu_inline_*/) {
> + WARN("PREFER_GNU_INLINE",
> + "__gnu_inline is preferred over __attribute__((__gnu_inline__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ malloc, prefer __malloc
> + if ($attr =~ /^_*malloc_*/) {
> + WARN("PREFER_MALLOC",
> + "__malloc is preferred over __attribute__((__malloc__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ mode, prefer __mode
> + if ($attr =~ /^_*mode_*/) {
> + WARN("PREFER_MODE",
> + "__mode(x) is preferred over __attribute__((__mode__(x)))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ no_caller_saved_registers, prefer __no_caller_saved_registers
> + if ($attr =~ /^_*no_caller_saved_registers_*/) {
> + WARN("PREFER_NO_CALLER_SAVED_REGISTERS",
> + "__no_caller_saved_registers is preferred over __attribute__((__no_caller_saved_registers__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ noclone, prefer __noclone
> + if ($attr =~ /^_*noclone_*/) {
> + WARN("PREFER_NOCLONE",
> + "__noclone is preferred over __attribute__((__noclone__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ fallthrough, prefer fallthrough
> + if ($attr =~ /^_*fallthrough_*/) {
> + WARN("PREFER_FALLTHROUGH",
> + "fallthrough is preferred over __attribute__((__fallthrough__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ noinline, prefer noinline
> + if ($attr =~ /^_*noinline_*/) {
> + WARN("PREFER_NOINLINE",
> + "noinline is preferred over __attribute__((__noinline__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ nonstring, prefer __nonstring
> + if ($attr =~ /^_*nonstring_*/) {
> + WARN("PREFER_NONSTRING",
> + "__nonstring is preferred over __attribute__((__nonstring__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ noreturn, prefer __noreturn
> + if ($attr =~ /^_*noreturn_*/) {
> + WARN("PREFER_NORETURN",
> + "__noreturn is preferred over __attribute__((__noreturn__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ packed, prefer __packed
> + if ($attr =~ /^_*packed_*/) {
> + WARN("PREFER_PACKED",
> + "__packed is preferred over __attribute__((__packed__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ pure, prefer __pure
> + if ($attr =~ /^_*pure_*/) {
> + WARN("PREFER_PURE",
> + "__pure is preferred over __attribute__((__pure__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ section, prefer __section
> + if ($attr =~ /^_*section_*\s*\(\s*("[^"]*")/) {
> + my $old = substr($rawline, $-[1], $+[1] - $-[1]);
> + my $new = substr($old, 1, -1);
> + if (WARN("PREFER_SECTION",
> + "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*\)\s*\)\s*\)/__section($new)/;
> + }
> + }
> +
> + # Check for __attribute__ unused, prefer __always_unused or __maybe_unused
> + if ($attr =~ /^_*unused_*/) {
> + WARN("PREFER_UNUSED",
> + "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr);
> + }
> +
> + # Check for __attribute__ used, prefer __used
> + if ($attr =~ /^_*used_*/) {
> + WARN("PREFER_USED",
> + "__used is preferred over __attribute__((__used__))\n" . $herecurr);
> }
> }
>
> --
> 2.27.0
>
Hi Lukas,
This patch is followed from the discussion at:
https://lore.kernel.org/linux-kernel-mentees/3ec15b41754b01666d94b76ce51b9832c2dd577a.camel@perches.com/
checkpatch didn't check a lot of the attributes which are
declared in include/linux/compiler-attributes.h
Like __attribute__((__used__)) is defined as __used,
which is generally the preferred way unless there is
a reason not to.
So check for use of the __attribute__ definitions
and generate a warning if the shorthand is not used.
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2020-10-22 12:58 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-22 12:47 [Linux-kernel-mentees] [PATCH RFC] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
2020-10-22 12:57 ` Dwaipayan Ray [this message]
2020-10-22 14:20 ` Lukas Bulwahn
2020-10-22 14:29 ` Dwaipayan Ray
2020-10-22 14:52 Dwaipayan Ray
2020-10-22 14:59 ` Dwaipayan Ray
2020-10-22 16:14 ` Joe Perches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABJPP5AObU2e2u8Zcd1J7k80dSqGPYGXQ_0RQVduBogzg6XJSg@mail.gmail.com \
--to=dwaipayanray1@gmail.com \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=lukas.bulwahn@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.