* [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns
@ 2020-10-25 6:51 Dwaipayan Ray
2020-10-25 8:22 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Dwaipayan Ray @ 2020-10-25 6:51 UTC (permalink / raw)
To: joe; +Cc: dwaipayanray1, linux-kernel-mentees, linux-kernel
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.
New attributes which are now handled are:
__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__
__noinline__
__nonstring__
__noreturn__
__pure__
__unused__
__used__
Also add fixes for the generic attributes check.
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 | 107 ++++++++++++++++++++++++++++--------------
1 file changed, 71 insertions(+), 36 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7e505688257a..1b736e9042d2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6155,50 +6155,85 @@ 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 = (
+ "alias" => "__alias",
+ "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"
+ );
+
+ if ($attr =~ /^(\w+)\s*(${balanced_parens})?/) {
+ 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};
+ 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/;
+ }
+ }
+ }
-# 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__ 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__ 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 ($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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns
2020-10-25 6:51 [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
@ 2020-10-25 8:22 ` Joe Perches
2020-10-25 8:32 ` Dwaipayan Ray
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2020-10-25 8:22 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel
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.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns
2020-10-25 8:22 ` Joe Perches
@ 2020-10-25 8:32 ` Dwaipayan Ray
2020-10-25 8:38 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Dwaipayan Ray @ 2020-10-25 8:32 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel
On Sun, Oct 25, 2020 at 1:52 PM Joe Perches <joe@perches.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.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns
2020-10-25 8:32 ` Dwaipayan Ray
@ 2020-10-25 8:38 ` Joe Perches
2020-10-25 8:42 ` Dwaipayan Ray
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2020-10-25 8:38 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel
On Sun, 2020-10-25 at 14:02 +0530, Dwaipayan Ray wrote:
> 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.
For now, I think you should avoid both alias and section.
There are patches to convert both of these macros to remove
the stringify.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns
2020-10-25 8:38 ` Joe Perches
@ 2020-10-25 8:42 ` Dwaipayan Ray
2020-10-25 8:48 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Dwaipayan Ray @ 2020-10-25 8:42 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel
On Sun, Oct 25, 2020 at 2:08 PM Joe Perches <joe@perches.com> wrote:
>
> On Sun, 2020-10-25 at 14:02 +0530, Dwaipayan Ray wrote:
> > 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.
>
> For now, I think you should avoid both alias and section.
>
> There are patches to convert both of these macros to remove
> the stringify.
>
>
Okay I will do that.
And another quick question please.
Suppose we have non handled attributes in one such
multi attribute macro. (not present in our hash)
Like __attribute__((__packed, __something_not_handled))
For this case, do I skip the warning totally? Or something else?
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns
2020-10-25 8:42 ` Dwaipayan Ray
@ 2020-10-25 8:48 ` Joe Perches
0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2020-10-25 8:48 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel
On Sun, 2020-10-25 at 14:12 +0530, Dwaipayan Ray wrote:
> Suppose we have non handled attributes in one such
> multi attribute macro. (not present in our hash)
>
> Like __attribute__((__packed, __something_not_handled))
>
> For this case, do I skip the warning totally? Or something else?
Emit the warning, skip the fix unless all converted.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-25 13:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-25 6:51 [Linux-kernel-mentees] [PATCH v4] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
2020-10-25 8:22 ` Joe Perches
2020-10-25 8:32 ` Dwaipayan Ray
2020-10-25 8:38 ` Joe Perches
2020-10-25 8:42 ` Dwaipayan Ray
2020-10-25 8:48 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).