From: Dwaipayan Ray <dwaipayanray1@gmail.com> To: joe@perches.com Cc: linux-kernel-mentees@lists.linuxfoundation.org, dwaipayanray1@gmail.com, linux-kernel@vger.kernel.org, lukas.bulwahn@gmail.com Subject: [PATCH v5] checkpatch: extend attributes check to handle more patterns Date: Sun, 25 Oct 2020 15:45:37 +0530 [thread overview] Message-ID: <20201025101537.59133-1-dwaipayanray1@gmail.com> (raw) 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, section, 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 to the generic attributes check to substitute the correct conversions. New attributes which are now handled are: __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__ __noinline__ __nonstring__ __noreturn__ __pure__ __unused__ __used__ Declarations which contain multiple attributes like __attribute__((__packed__, __cold__)) are also handled except when proper conversions for one or more attributes of the list cannot be determined. Link: https://lore.kernel.org/linux-kernel-mentees/3ec15b41754b01666d94b76ce51b9832c2dd577a.camel@perches.com/ Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com> --- scripts/checkpatch.pl | 117 +++++++++++++++++++++++++++++------------- 1 file changed, 81 insertions(+), 36 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7e505688257a..e3437948883b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6155,50 +6155,95 @@ 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); - } + $rawline =~ /\b__attribute__\s*\(\s*($balanced_parens)\s*\)/) { + my $attr = $1; + $attr =~ s/\s*\(\s*(.*)\)\s*/$1/; + + my %attr_list = ( + "aligned" => "__aligned", + "always_inline" => "__always_inline", + "assume_aligned" => "__assume_aligned", + "cold" => "__cold", + "const" => "__const", + "copy" => "__copy", + "designated_init" => "__designated_init", + "externally_visible" => "__visible", + "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" + ); + + my @conv_array = (); + my $conv_possible = 1; + + while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { + my $curr_attr = $1; + my $params = ''; + $params = $2 if defined($2); + $curr_attr =~ s/^[\s_]+|[\s_]+$//g; + + if (exists($attr_list{$curr_attr})) { + my $new = $attr_list{$curr_attr}; + push(@conv_array, "$new$params"); + } else { + $conv_possible = 0; + last; + } + } -# 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); - } + if (scalar @conv_array > 0 && $conv_possible == 1) { + my $replace = join(' ', @conv_array); + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "$replace is preferred over __attribute__(($attr))\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*\Q$attr\E\s*\)\s*\)/$replace/; + } + } -# 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__ format(printf, prefer __printf + if ($attr =~ /^_*format_*\s*\(\s*printf/) { + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__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(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__ format(scanf, prefer __scanf + if ($attr =~ /^_*format_*\s*\(\s*scanf\b/) { + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__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__ section, prefer __section + if ($attr =~ /^_*section_*\s*\(\s*("[^"]*")/) { + my $old = substr($attr, $-[1], $+[1] - $-[1]); + my $new = substr($old, 1, -1); + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__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__ 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__ unused, prefer __always_unused or __maybe_unused + if ($attr =~ /^_*unused/) { + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr); } } -- 2.27.0
WARNING: multiple messages have this Message-ID (diff)
From: Dwaipayan Ray <dwaipayanray1@gmail.com> To: joe@perches.com Cc: dwaipayanray1@gmail.com, linux-kernel-mentees@lists.linuxfoundation.org, linux-kernel@vger.kernel.org Subject: [Linux-kernel-mentees] [PATCH v5] checkpatch: extend attributes check to handle more patterns Date: Sun, 25 Oct 2020 15:45:37 +0530 [thread overview] Message-ID: <20201025101537.59133-1-dwaipayanray1@gmail.com> (raw) 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, section, 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 to the generic attributes check to substitute the correct conversions. New attributes which are now handled are: __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__ __noinline__ __nonstring__ __noreturn__ __pure__ __unused__ __used__ Declarations which contain multiple attributes like __attribute__((__packed__, __cold__)) are also handled except when proper conversions for one or more attributes of the list cannot be determined. Link: https://lore.kernel.org/linux-kernel-mentees/3ec15b41754b01666d94b76ce51b9832c2dd577a.camel@perches.com/ Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com> --- scripts/checkpatch.pl | 117 +++++++++++++++++++++++++++++------------- 1 file changed, 81 insertions(+), 36 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7e505688257a..e3437948883b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6155,50 +6155,95 @@ 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); - } + $rawline =~ /\b__attribute__\s*\(\s*($balanced_parens)\s*\)/) { + my $attr = $1; + $attr =~ s/\s*\(\s*(.*)\)\s*/$1/; + + my %attr_list = ( + "aligned" => "__aligned", + "always_inline" => "__always_inline", + "assume_aligned" => "__assume_aligned", + "cold" => "__cold", + "const" => "__const", + "copy" => "__copy", + "designated_init" => "__designated_init", + "externally_visible" => "__visible", + "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" + ); + + my @conv_array = (); + my $conv_possible = 1; + + while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { + my $curr_attr = $1; + my $params = ''; + $params = $2 if defined($2); + $curr_attr =~ s/^[\s_]+|[\s_]+$//g; + + if (exists($attr_list{$curr_attr})) { + my $new = $attr_list{$curr_attr}; + push(@conv_array, "$new$params"); + } else { + $conv_possible = 0; + last; + } + } -# 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); - } + if (scalar @conv_array > 0 && $conv_possible == 1) { + my $replace = join(' ', @conv_array); + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "$replace is preferred over __attribute__(($attr))\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*\Q$attr\E\s*\)\s*\)/$replace/; + } + } -# 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__ format(printf, prefer __printf + if ($attr =~ /^_*format_*\s*\(\s*printf/) { + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__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(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__ format(scanf, prefer __scanf + if ($attr =~ /^_*format_*\s*\(\s*scanf\b/) { + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__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__ section, prefer __section + if ($attr =~ /^_*section_*\s*\(\s*("[^"]*")/) { + my $old = substr($attr, $-[1], $+[1] - $-[1]); + my $new = substr($old, 1, -1); + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__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__ 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__ unused, prefer __always_unused or __maybe_unused + if ($attr =~ /^_*unused/) { + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr); } } -- 2.27.0 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next reply other threads:[~2020-10-25 10:16 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-25 10:15 Dwaipayan Ray [this message] 2020-10-25 10:15 ` [Linux-kernel-mentees] [PATCH v5] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray 2020-10-25 17:59 ` Joe Perches 2020-10-25 17:59 ` [Linux-kernel-mentees] " Joe Perches 2020-10-25 18:10 ` Dwaipayan Ray 2020-10-25 18:10 ` [Linux-kernel-mentees] " Dwaipayan Ray 2020-10-25 18:19 ` Joe Perches 2020-10-25 18:19 ` [Linux-kernel-mentees] " Joe Perches 2020-10-25 18:26 ` Dwaipayan Ray 2020-10-25 18:26 ` [Linux-kernel-mentees] " Dwaipayan Ray 2020-10-25 18:48 ` Dwaipayan Ray 2020-10-25 18:48 ` [Linux-kernel-mentees] " Dwaipayan Ray
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=20201025101537.59133-1-dwaipayanray1@gmail.com \ --to=dwaipayanray1@gmail.com \ --cc=joe@perches.com \ --cc=linux-kernel-mentees@lists.linuxfoundation.org \ --cc=linux-kernel@vger.kernel.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: linkBe 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.