* [PATCH v3] checkpatch: extend attributes check to handle more patterns
@ 2020-10-24 9:05 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-10-24 9:05 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, 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__
__fallthrough__
__noinline__
__nonstring__
__noreturn__
__pure__
__unused__
__used__
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 | 105 +++++++++++++++++++++++++++---------------
1 file changed, 69 insertions(+), 36 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7e505688257a..01d83d345b4c 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);
- }
+ $line =~ /__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",
+ "fallthrough" => "fallthrough",
+ "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};
+ WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+ "$new$params is preffered over __attribute__(($attr))\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__ 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($rawline, $-[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 related [flat|nested] 8+ messages in thread
* [Linux-kernel-mentees] [PATCH v3] checkpatch: extend attributes check to handle more patterns
@ 2020-10-24 9:05 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-10-24 9:05 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__
__fallthrough__
__noinline__
__nonstring__
__noreturn__
__pure__
__unused__
__used__
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 | 105 +++++++++++++++++++++++++++---------------
1 file changed, 69 insertions(+), 36 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7e505688257a..01d83d345b4c 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);
- }
+ $line =~ /__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",
+ "fallthrough" => "fallthrough",
+ "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};
+ WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+ "$new$params is preffered over __attribute__(($attr))\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__ 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($rawline, $-[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] 8+ messages in thread
* Re: [PATCH v3] checkpatch: extend attributes check to handle more patterns
2020-10-24 9:05 ` [Linux-kernel-mentees] " Dwaipayan Ray
@ 2020-10-24 20:20 ` Dwaipayan Ray
-1 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-10-24 20:20 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
On Sat, Oct 24, 2020 at 2:36 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 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__
> __fallthrough__
> __noinline__
> __nonstring__
> __noreturn__
> __pure__
> __unused__
> __used__
>
> 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 | 105 +++++++++++++++++++++++++++---------------
> 1 file changed, 69 insertions(+), 36 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7e505688257a..01d83d345b4c 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);
> - }
> + $line =~ /__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",
> + "fallthrough" => "fallthrough",
> + "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};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new$params is preffered over __attribute__(($attr))\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__ 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($rawline, $-[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
>
Hi,
In this version there is only one generic test for both parameterized
and non parameterized attributes.
And four attribute checks are kept separate and unaltered from the
original implementation. Those are the format(printf/scanf), section
and unused(which can be resolved to __always_unused or __maybe_unused).
Thanks,
Dwaipayan.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v3] checkpatch: extend attributes check to handle more patterns
@ 2020-10-24 20:20 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-10-24 20:20 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel
On Sat, Oct 24, 2020 at 2:36 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 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__
> __fallthrough__
> __noinline__
> __nonstring__
> __noreturn__
> __pure__
> __unused__
> __used__
>
> 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 | 105 +++++++++++++++++++++++++++---------------
> 1 file changed, 69 insertions(+), 36 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7e505688257a..01d83d345b4c 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);
> - }
> + $line =~ /__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",
> + "fallthrough" => "fallthrough",
> + "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};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new$params is preffered over __attribute__(($attr))\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__ 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($rawline, $-[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
>
Hi,
In this version there is only one generic test for both parameterized
and non parameterized attributes.
And four attribute checks are kept separate and unaltered from the
original implementation. Those are the format(printf/scanf), section
and unused(which can be resolved to __always_unused or __maybe_unused).
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] 8+ messages in thread
* Re: [PATCH v3] checkpatch: extend attributes check to handle more patterns
2020-10-24 9:05 ` [Linux-kernel-mentees] " Dwaipayan Ray
@ 2020-10-24 23:21 ` Joe Perches
-1 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-10-24 23:21 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On Sat, 2020-10-24 at 14:35 +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.
[]
> - }
> + $line =~ /__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",
> + "fallthrough" => "fallthrough",
I'd remove fallthrough.
It doesn't make sense as the attribute could be in any line
of a switch/case block and fallthrough; must be the last line
of the block.
> + 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};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new$params is preffered over __attribute__(($attr))\n" . $herecurr);
Be nice to have a $fix option here
> + # 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;
like for format(printf, index, pos)
and format(scanf, index, pos)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v3] checkpatch: extend attributes check to handle more patterns
@ 2020-10-24 23:21 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-10-24 23:21 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel
On Sat, 2020-10-24 at 14:35 +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.
[]
> - }
> + $line =~ /__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",
> + "fallthrough" => "fallthrough",
I'd remove fallthrough.
It doesn't make sense as the attribute could be in any line
of a switch/case block and fallthrough; must be the last line
of the block.
> + 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};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new$params is preffered over __attribute__(($attr))\n" . $herecurr);
Be nice to have a $fix option here
> + # 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;
like for format(printf, index, pos)
and format(scanf, index, pos)
_______________________________________________
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] 8+ messages in thread
* Re: [PATCH v3] checkpatch: extend attributes check to handle more patterns
2020-10-24 23:21 ` [Linux-kernel-mentees] " Joe Perches
@ 2020-10-24 23:29 ` Randy Dunlap
-1 siblings, 0 replies; 8+ messages in thread
From: Randy Dunlap @ 2020-10-24 23:29 UTC (permalink / raw)
To: Joe Perches, Dwaipayan Ray
Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On 10/24/20 4:21 PM, Joe Perches wrote:
>> + if (exists($attr_list{$curr_attr})) {
>> + my $new = $attr_list{$curr_attr};
>> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
>> + "$new$params is preffered over __attribute__(($attr))\n" . $herecurr);
preferred
Sorry if that has already been mentioned.
--
~Randy
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v3] checkpatch: extend attributes check to handle more patterns
@ 2020-10-24 23:29 ` Randy Dunlap
0 siblings, 0 replies; 8+ messages in thread
From: Randy Dunlap @ 2020-10-24 23:29 UTC (permalink / raw)
To: Joe Perches, Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel
On 10/24/20 4:21 PM, Joe Perches wrote:
>> + if (exists($attr_list{$curr_attr})) {
>> + my $new = $attr_list{$curr_attr};
>> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
>> + "$new$params is preffered over __attribute__(($attr))\n" . $herecurr);
preferred
Sorry if that has already been mentioned.
--
~Randy
_______________________________________________
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] 8+ messages in thread
end of thread, other threads:[~2020-10-24 23:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-24 9:05 [PATCH v3] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
2020-10-24 9:05 ` [Linux-kernel-mentees] " Dwaipayan Ray
2020-10-24 20:20 ` Dwaipayan Ray
2020-10-24 20:20 ` [Linux-kernel-mentees] " Dwaipayan Ray
2020-10-24 23:21 ` Joe Perches
2020-10-24 23:21 ` [Linux-kernel-mentees] " Joe Perches
2020-10-24 23:29 ` Randy Dunlap
2020-10-24 23:29 ` [Linux-kernel-mentees] " Randy Dunlap
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.