* [PATCH v6] checkpatch: extend attributes check to handle more patterns
@ 2020-10-25 19:31 Dwaipayan Ray
2020-10-25 19:36 ` Joe Perches
2020-10-27 22:45 ` [PATCH -next] checkpatch: Update __attribute__((section("name"))) quote removal Joe Perches
0 siblings, 2 replies; 3+ messages in thread
From: Dwaipayan Ray @ 2020-10-25 19:31 UTC (permalink / raw)
To: joe; +Cc: linux-kernel-mentees, dwaipayanray1, linux-kernel, lukas.bulwahn
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 | 109 +++++++++++++++++++++++++++---------------
1 file changed, 71 insertions(+), 38 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7e505688257a..eeebd8149be9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6155,50 +6155,83 @@ 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);
- }
-
-# 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__ 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)/;
+ $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" => "__attribute_const__",
+ "copy" => "__copy",
+ "designated_init" => "__designated_init",
+ "externally_visible" => "__visible",
+ "format" => "printf|scanf",
+ "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})) {
+ if ($curr_attr eq "format" && $params) {
+ $params =~ /^\s*\(\s*(\w+)\s*,\s*(.*)/;
+ push(@conv_array, "__$1\($2");
+ } else {
+ my $new = $attr_list{$curr_attr};
+ push(@conv_array, "$new$params");
+ }
+ } else {
+ $conv_possible = 0;
+ last;
+ }
}
- }
-# 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;
+ 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/;
+ $fixed[$fixlinenr] =~ s/\}\Q$replace\E/} $replace/;
+ }
+ }
+ # 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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v6] checkpatch: extend attributes check to handle more patterns
2020-10-25 19:31 [PATCH v6] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
@ 2020-10-25 19:36 ` Joe Perches
2020-10-27 22:45 ` [PATCH -next] checkpatch: Update __attribute__((section("name"))) quote removal Joe Perches
1 sibling, 0 replies; 3+ messages in thread
From: Joe Perches @ 2020-10-25 19:36 UTC (permalink / raw)
To: Dwaipayan Ray, Andrew Morton
Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On Mon, 2020-10-26 at 01:01 +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, 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.
Thanks Dwaipayan.
Andrew, can you pick this up please?
Link: https://lore.kernel.org/lkml/20201025193103.23223-1-dwaipayanray1@gmail.com/raw
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH -next] checkpatch: Update __attribute__((section("name"))) quote removal
2020-10-25 19:31 [PATCH v6] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
2020-10-25 19:36 ` Joe Perches
@ 2020-10-27 22:45 ` Joe Perches
1 sibling, 0 replies; 3+ messages in thread
From: Joe Perches @ 2020-10-27 22:45 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray
commit 33def8498fdd ("treewide: Convert macro and uses of
__section(foo) to __section("foo")") removed the stringification of the
section name and now requires quotes around the named section.
Update checkpatch to not remove any quotes when suggesting conversion
of __attribute__((section("name"))) to __section("name")
Miscellanea:
o Add section to the hash with __section replacement
o Remove separate test for __attribute__((section
o Remove the limitation on converting attributes containing only
known, possible conversions. Any unknown attribute types are now
left as-is and known types are converted and moved before
__attribute__ and removed from within the __attribute__((list...)).
Signed-off-by: Joe Perches <joe@perches.com>
---
scripts/checkpatch.pl | 46 ++++++++++++++++------------------------------
1 file changed, 16 insertions(+), 30 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7800a090e8fe..a97f2ab11568 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6216,51 +6216,37 @@ sub process {
"noreturn" => "__noreturn",
"packed" => "__packed",
"pure" => "__pure",
+ "section" => "__section",
"used" => "__used"
);
- my @conv_array = ();
- my $conv_possible = 1;
-
while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) {
- my $curr_attr = $1;
+ my $orig_attr = $1;
my $params = '';
$params = $2 if defined($2);
+ my $curr_attr = $orig_attr;
$curr_attr =~ s/^[\s_]+|[\s_]+$//g;
-
if (exists($attr_list{$curr_attr})) {
+ my $new = $attr_list{$curr_attr};
if ($curr_attr eq "format" && $params) {
$params =~ /^\s*\(\s*(\w+)\s*,\s*(.*)/;
- push(@conv_array, "__$1\($2");
+ $new = "__$1\($2";
} else {
- my $new = $attr_list{$curr_attr};
- push(@conv_array, "$new$params");
+ $new = "$new$params";
+ }
+ if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+ "Prefer $new over __attribute__(($orig_attr$params))\n" . $herecurr) &&
+ $fix) {
+ my $remove = "\Q$orig_attr\E" . '\s*' . "\Q$params\E" . '(?:\s*,\s*)?';
+ $fixed[$fixlinenr] =~ s/$remove//;
+ $fixed[$fixlinenr] =~ s/\b__attribute__/$new __attribute__/;
+ $fixed[$fixlinenr] =~ s/\}\Q$new\E/} $new/;
}
- } else {
- $conv_possible = 0;
- last;
- }
- }
-
- 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/;
- $fixed[$fixlinenr] =~ s/\}\Q$replace\E/} $replace/;
}
}
- # 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)/;
- }
+ if (show_type("PREFER_DEFINED_ATTRIBUTE_MACRO") && $fix) {
+ $fixed[$fixlinenr] =~ s/ __attribute__\s*\(\s*\(\s*\)\s*\)//;
}
# Check for __attribute__ unused, prefer __always_unused or __maybe_unused
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-27 22:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-25 19:31 [PATCH v6] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
2020-10-25 19:36 ` Joe Perches
2020-10-27 22:45 ` [PATCH -next] checkpatch: Update __attribute__((section("name"))) quote removal Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®