* [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
@ 2020-10-23 9:43 Dwaipayan Ray
2020-10-23 11:04 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Dwaipayan Ray @ 2020-10-23 9:43 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 | 130 ++++++++++++++++++++++++++++++------------
1 file changed, 94 insertions(+), 36 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7e505688257a..69324c2e55d6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6155,50 +6155,108 @@ 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);
+
+ my $attr_list = qr {
+ __alias__|
+ __aligned__$|
+ __aligned__\(.*\)|
+ __always_inline__|
+ __assume_aligned__\(.*\)|
+ __cold__|
+ __const__|
+ __copy__\(.*\)|
+ __designated_init__|
+ __externally_visible__|
+ __fallthrough__|
+ __gnu_inline__|
+ __malloc__|
+ __mode__\(.*\)|
+ __no_caller_saved_registers__|
+ __noclone__|
+ __noinline__|
+ __nonstring__|
+ __noreturn__|
+ __packed__|
+ __pure__|
+ __used__
+ }x;
-# 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);
- }
+ my %attr_replace = (
+ "__alias__" => "__alias",
+ "__aligned__" => "__aligned_largest",
+ "__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 =~/^($attr_list)/) {
+ my $old = $1;
+ if ($old =~ /^(__.+__\()(.*)\)/) {
+ my $new = $attr_replace{$1};
+ WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+ "$new($2) is preffered over __attribute__(($old))\n" . $herecurr);
+ } else {
+ my $new = $attr_replace{$old};
+ WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+ "$new is preffered over __attribute__(($old))\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__ 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($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] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 9:43 [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
@ 2020-10-23 11:04 ` Joe Perches
2020-10-23 11:40 ` Dwaipayan Ray
0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2020-10-23 11:04 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On Fri, 2020-10-23 at 15:13 +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
checkpatch, no need for capitalization
and non-trivially:
> + my $attr_list = qr {
> + __alias__|
> + __aligned__$|
> + __aligned__\(.*\)|
> + __always_inline__|
> + __assume_aligned__\(.*\)|
> + __cold__|
> + __const__|
> + __copy__\(.*\)|
> + __designated_init__|
> + __externally_visible__|
> + __fallthrough__|
> + __gnu_inline__|
> + __malloc__|
> + __mode__\(.*\)|
> + __no_caller_saved_registers__|
> + __noclone__|
> + __noinline__|
> + __nonstring__|
> + __noreturn__|
> + __packed__|
> + __pure__|
> + __used__
> + }x;
[]
> + my %attr_replace = (
> + "__alias__" => "__alias",
> + "__aligned__" => "__aligned_largest",
> + "__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 =~/^($attr_list)/) {
I would remove the __ from the entries here.
And you could check using
$line =~ /__attribute__\s*\(\s*($balanced_parens)\s*)/
and check for all attributes in $1 after
stripping the leading and trailing parens
and any leading and trailing underscores
from each attribute.
And you only need one hash and you should
check for existence of the key rather than
have multiple lists.
if (exists($attributes($attr))) {
> + my $old = $1;
> + if ($old =~ /^(__.+__\()(.*)\)/) {
> + my $new = $attr_replace{$1};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new($2) is preffered over __attribute__(($old))\n" . $herecurr);
preferred
> + } else {
> + my $new = $attr_replace{$old};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new is preffered over __attribute__(($old))\n" . $herecurr);
> + }
> + }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 11:04 ` Joe Perches
@ 2020-10-23 11:40 ` Dwaipayan Ray
2020-10-23 12:08 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Dwaipayan Ray @ 2020-10-23 11:40 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
On Fri, Oct 23, 2020 at 4:34 PM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2020-10-23 at 15:13 +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
>
> checkpatch, no need for capitalization
>
> and non-trivially:
>
> > + my $attr_list = qr {
> > + __alias__|
> > + __aligned__$|
> > + __aligned__\(.*\)|
> > + __always_inline__|
> > + __assume_aligned__\(.*\)|
> > + __cold__|
> > + __const__|
> > + __copy__\(.*\)|
> > + __designated_init__|
> > + __externally_visible__|
> > + __fallthrough__|
> > + __gnu_inline__|
> > + __malloc__|
> > + __mode__\(.*\)|
> > + __no_caller_saved_registers__|
> > + __noclone__|
> > + __noinline__|
> > + __nonstring__|
> > + __noreturn__|
> > + __packed__|
> > + __pure__|
> > + __used__
> > + }x;
> []
> > + my %attr_replace = (
> > + "__alias__" => "__alias",
> > + "__aligned__" => "__aligned_largest",
> > + "__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 =~/^($attr_list)/) {
>
> I would remove the __ from the entries here.
>
> And you could check using
>
> $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*)/
>
> and check for all attributes in $1 after
> stripping the leading and trailing parens
> and any leading and trailing underscores
> from each attribute.
>
> And you only need one hash and you should
> check for existence of the key rather than
> have multiple lists.
>
> if (exists($attributes($attr))) {
>
Okay thanks!
But what should be done for the attributes which are
parameterized, like __aligned__(x). Should all the __
for these entries be trimmed too? There are also
cases where there are multiple versions like:
__aligned__
__aligned__(x)
To help differentiate between them what can be done?
Should i make the keys as:
aligned
aligned__(
instead of
__aligned__
__aligned__(
Thanks,
Dwaipayan.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 11:40 ` Dwaipayan Ray
@ 2020-10-23 12:08 ` Joe Perches
2020-10-23 19:14 ` Dwaipayan Ray
0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2020-10-23 12:08 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
On Fri, 2020-10-23 at 17:10 +0530, Dwaipayan Ray wrote:
> On Fri, Oct 23, 2020 at 4:34 PM Joe Perches <joe@perches.com> wrote:
> >
> > On Fri, 2020-10-23 at 15:13 +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
> >
> > checkpatch, no need for capitalization
> >
> > and non-trivially:
> >
> > > + my $attr_list = qr {
> > > + __alias__|
> > > + __aligned__$|
> > > + __aligned__\(.*\)|
> > > + __always_inline__|
> > > + __assume_aligned__\(.*\)|
> > > + __cold__|
> > > + __const__|
> > > + __copy__\(.*\)|
> > > + __designated_init__|
> > > + __externally_visible__|
> > > + __fallthrough__|
> > > + __gnu_inline__|
> > > + __malloc__|
> > > + __mode__\(.*\)|
> > > + __no_caller_saved_registers__|
> > > + __noclone__|
> > > + __noinline__|
> > > + __nonstring__|
> > > + __noreturn__|
> > > + __packed__|
> > > + __pure__|
> > > + __used__
> > > + }x;
> > []
> > > + my %attr_replace = (
> > > + "__alias__" => "__alias",
> > > + "__aligned__" => "__aligned_largest",
> > > + "__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 =~/^($attr_list)/) {
> >
> > I would remove the __ from the entries here.
> >
> > And you could check using
> >
> > $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*)/
> >
> > and check for all attributes in $1 after
> > stripping the leading and trailing parens
> > and any leading and trailing underscores
> > from each attribute.
> >
> > And you only need one hash and you should
> > check for existence of the key rather than
> > have multiple lists.
> >
> > if (exists($attributes($attr))) {
> >
>
> Okay thanks!
> But what should be done for the attributes which are
> parameterized, like __aligned__(x). Should all the __
> for these entries be trimmed too?
yes
> There are also
> cases where there are multiple versions like:
>
> __aligned__
> __aligned__(x)
$ git grep __aligned__ | grep -v -P '__aligned__\s*\('
AFAIK: There is only one use of bare __aligned__ and
that's in compiler_attributes.h
> To help differentiate between them what can be done?
> Should i make the keys as:
>
> aligned
> aligned__(
>
> instead of
>
> __aligned__
> __aligned__(
Just use aligned
Just fyi:
these are the uses of __attribute__ in the kernel
with all the underscores and spaces removed so there's
some value in finding the multiple actual attributes .
$ git grep -ohP '__attribute__\s*\(\s*\(.*\)\s*\)' | \
sed -r -e 's/\s//g' -r -e 's/__(\w+)__/\1/g' | \
sort | uniq -c | sort -rn
2181 attribute((packed))
212 attribute((unused))
206 attribute((packed,aligned(4)))
131 attribute((aligned(8)))
77 attribute((aligned(32)))
74 attribute((always_inline))
68 attribute((noinline))
52 attribute((preserve_access_index))
48 attribute((weak))
38 attribute((noreturn))
38 attribute((aligned(16)))
25 attribute((packed,aligned(8)))
25 attribute((const))
18 attribute((aligned(64)))
17 attribute((aligned(256)))
17 attribute((aligned(128)))
16 attribute((used))
16 attribute((format(printf,1,2)))
15 attribute((aligned(4)))
14 attribute((constructor))
10 attribute((unused)))
10 attribute((pure))
10 attribute((mode(DI)))
10 attribute((aligned(SMP_CACHE_BYTES)))
9 attribute((format(printf,4,5)))
9 attribute((format(printf,2,3)))
9 attribute((aligned(sizeof(long))))
8 attribute((mode(SI)))
8 attribute((aligned(PAGE_SIZE)))
7 attribute((visibility("hidden")))
7 attribute((packed,aligned(2)))
6 attribute((section(NAME),used))
6 attribute((section("_ftrace_events")))
6 attribute((section(".data..read_mostly")))
6 attribute((section(".data")))
6 attribute((mode(word)))
5 attribute((format(printf,a,b)))
5 attribute((always_inline))typeof(name(0))
5 attribute((aligned(sizeof(unsignedlong))))
5 attribute((aligned(alignof(structebt_replace))))
5 attribute((aligned(AESNI_ALIGN)))
4 attribute((weak,alias("__vdso_clock_gettime")))
4 attribute((section("__trace_printk_fmt")))
4 attribute((section(".inittext")))
4 attribute((packed,aligned(256)))
4 attribute((packed,aligned(16)))
4 attribute((nonnull))
4 attribute((may_alias))
4 attribute((format(printf,3,4)))
4 attribute((fallthrough))
4 attribute((aligned(x)))
4 attribute((aligned(sizeof(void*))))
4 attribute((aligned(BITS_PER_LONG/8)))
4 attribute((aligned(alignof(u64))))
3 attribute((weak,alias("__vdso_gettimeofday")))
3 attribute((visibility("default")))
3 attribute((section("data_sec")))
3 attribute((regparm(0)))
3 attribute((nonnull(2,3)))
3 attribute((mode(TI)))
3 attribute((flatten))
3 attribute((destructor))
3 attribute((bitwise))
3 attribute((aligned(sizeof(uint64_t))))
3 attribute((aligned(sizeof(__u64))))
3 attribute((aligned(PADLOCK_ALIGNMENT)))
3 attribute((aligned(L1_CACHE_BYTES)))
3 attribute((aligned(a)))
3 attribute((alias("thermal_genl_event_tz")))
2 attribute((weak,unused))
2 attribute((weak,section(".rodata")))
2 attribute((weak,alias("__vdso_time")))
2 attribute((weak,alias("__vdso_getcpu")))
2 attribute((weak,alias("__vdso_clock_getres")))
2 attribute((warn_unused_result,nonnull))
2 attribute((vector_size(16)))
2 attribute((user))
2 attribute((section(".__syscall_stub")))
2 attribute((section(".maps."#name),used))
2 attribute((section("license"),used))
2 attribute((section("_ftrace_eval_map")))
2 attribute((section(".cpuidle.text")))
2 attribute((section("__builtin_cmdline")))
2 attribute((section(".arch.info.init")))
2 attribute((packed,aligned(PMCRAID_IOARCB_ALIGNMENT)))
2 attribute((packed,aligned(64)))
2 attribute((no_sanitize_address))
2 attribute((noreturn,unused))
2 attribute((nonnull(2)))
2 attribute((no_instrument_function))
2 attribute((model(small)))
2 attribute((malloc))
2 attribute((latent_entropy))
2 attribute((gnu_inline))
2 attribute((format(scanf,a,b)))
2 attribute((format(printf,1,2)))attribute((noreturn))
2 attribute((force))
2 attribute((externally_visible))
2 attribute((error(message)))
2 attribute((common))
2 attribute((aligned(XCHAL_NCP_SA_ALIGN)))
2 attribute((aligned(VRING_USED_ALIGN_SIZE)))
2 attribute((aligned((sizeof(long)))))
2 attribute((aligned(sizeof(Elf##size##_Word))))
2 attribute((aligned(65536)))
2 attribute((aligned(512)))
2 attribute((aligned(4*sizeof(__u64))))
2 attribute((aligned(32),packed))
2 attribute((aligned(0x20000)))
2 attribute((alias(__stringify(__se_sys##name))))
2 attribute((alias(__stringify(__se_compat_sys##name))))
2 attribute((alias("cfi_cmdset_0002")))
2 attribute((alias("cfi_cmdset_0001")))
1 attribute((weakref("dsdt_aml_code")))
1 attribute((weakref("AmlCode")))
1 attribute((weak,alias("__vdso_clock_gettime64")))
1 attribute((weak,alias("__platform_"#f)))
1 attribute((weak,alias("__mips_cm_phys_base")))
1 attribute((weak,alias("__mips_cm_l2sync_phys_base")))
1 attribute((weak,alias("__alloc_bootmem_huge_page")))
1 attribute((warn_unused_result))
1 attribute((warning(message)))
1 attribute((vector_size(8)))
1 attribute((used,section(".videocards")))
1 attribute((used,section(".taglist")))
1 attribute((unused,section("__param"),aligned(sizeof(void*))))
1 attribute((unused,alias(__stringify(name))))
1 attribute((tls_model("initial-exec"),unused))
1 attribute((tls_model("initial-exec")))
1 attribute((target("no-vsx")))
1 attribute((syscall_linkage))
1 attribute((section(".xiptext")))
1 attribute((section(".x86_intel_mid_dev.init")))
1 attribute((section(".x86_cpu_dev.init")))
1 attribute((section(".vvar_"#name),aligned(16)))
1 attribute((section(".text.hot")))
1 attribute((section(".taglist.init")))
1 attribute((section("__syscalls_metadata")))
1 attribute((section("__stop_sched_class")))
1 attribute((section(".spinlock.text")))
1 attribute((section(".softirqentry.text")))
1 attribute((section(#section),aligned((sizeof(void*)))))
1 attribute((section(#__sec".init")))
1 attribute((section(".sched.text")))
1 attribute((section(#S)))
1 attribute((section("__rt_sched_class")))
1 attribute((section(PER_CPU_BASE_SECTIONsec)))
1 attribute((section("__modver")))
1 attribute((section(".modinfo"),unused,aligned(1)))
1 attribute((section(".machine.desc")))
1 attribute((section("___ksymtab"sec"+"#sym),used))
1 attribute((section(".ksyms")))
1 attribute((section(".kprobes.text")))
1 attribute((section("_kprobe_blacklist")))
1 attribute((section("___kentry+"#sym)))
1 attribute((section(".kconfig")))
1 attribute((section(".irqentry.text")))
1 attribute((section(".initcall"level".init")))
1 attribute((section("__idle_sched_class")))
1 attribute((section("__fair_sched_class")))
1 attribute((section("_error_injection_whitelist")))
1 attribute((section("__dl_sched_class")))
1 attribute((section(".discard"),unused))
1 attribute((section(".data..vm0.pte"),aligned(PAGE_SIZE)))
1 attribute((section(".data..vm0.pmd"),aligned(PAGE_SIZE)))
1 attribute((section(".data..vm0.pgd"),aligned(PAGE_SIZE)))
1 attribute((section(".data..ro_after_init")))
1 attribute((section(".data..init_thread_info")))
1 attribute((section(".data..init_task")))
1 attribute((section(".data..init_irqstack")))
1 attribute((section(".bss..decrypted")))
1 attribute((section("__bpf_raw_tp_map")))
1 attribute((section("action-ok"),used))
1 attribute((section("action-ko"),used))
1 attribute((section($old)))
1 attribute((safe))
1 attribute((returns_twice))
1 attribute((require_context(x,1,999,"write")))
1 attribute((require_context(x,1,999,"read")))
1 attribute((require_context(x,1,999,"rdwr")))
1 attribute((randomize_layout))
1 attribute((patchable_function_entry(0,0)))
1 attribute((packed,deprecated))
1 attribute((packed,aligned(XSAVE_ALIGNMENT)))
1 attribute((packed,aligned(PMCRAID_IOADL_ALIGNMENT)))
1 attribute((packed,aligned(PAGE_SIZE)))
1 attribute((packed,aligned(2048)))
1 attribute((packed,aligned(1024)))
1 attribute((optimize("no-stack-protector")))
1 attribute((optimize("no-optimize-sibling-calls")))
1 attribute((optimize("-fno-gcse")))
1 attribute((no_sanitize_undefined))
1 attribute((no_sanitize("undefined")))
1 attribute((no_sanitize_thread))
1 attribute((no_sanitize("thread")))
1 attribute((no_sanitize("shadow-call-stack")))
1 attribute((no_sanitize("address","hwaddress")))
1 attribute((noreturn))__attribute((format(printf,1,2)))
1 attribute((no_randomize_layout))
1 attribute((nonstring))
1 attribute((nonnull(2,3,4,5)))
1 attribute((nonnull(2,3,4)))
1 attribute((nonnull(1)))
1 attribute((noderef,address_space(__user)))
1 attribute((noderef,address_space(__rcu)))
1 attribute((noderef,address_space(__percpu)))
1 attribute((noderef,address_space(__iomem)))
1 attribute((noderef))
1 attribute((noclone))
1 attribute((nocast))
1 attribute((no_caller_saved_registers))
1 attribute((name))
1 attribute((naked))
1 attribute((ms_abi))
1 attribute((mode(x)))
1 attribute((mode(TF)))
1 attribute((mode(QI)))
1 attribute((mode(HI)))
1 attribute((long_call))
1 attribute((indirect_branch("keep")))
1 attribute((hotpatch(0,0)))
1 attribute((hot))
1 attribute((format(scanf,string-index,first-to-check)))
1 attribute((format(printf,string-index,first-to-check)))
1 attribute((format(printf,(pos_fmtstr),(pos_fmtargs))))
1 attribute((format(printf,i,j)))
1 attribute((format(printf,c,c+1)))
1 attribute((format(printf,5,6)))
1 attribute((format(printf,2,0)))
1 attribute((format(gnu_printf,i,j)))
1 attribute((disable_tail_calls))
1 attribute((designated_init))
1 attribute((deprecated(msg)))
1 attribute((deprecated))
1 attribute((copy(symbol)))
1 attribute((context(x,1,1)))
1 attribute((context(x,1,0)))
1 attribute((context(x,0,1)))
1 attribute((cold))
1 attribute((cleanup(kcsan_end_scoped_access)))
1 attribute((assume_aligned(a,##VA_ARGS)))
1 attribute((align_value(8)))
1 attribute((aligned(XCHAL_CP7_SA_ALIGN)))
1 attribute((aligned(XCHAL_CP6_SA_ALIGN)))
1 attribute((aligned(XCHAL_CP5_SA_ALIGN)))
1 attribute((aligned(XCHAL_CP4_SA_ALIGN)))
1 attribute((aligned(XCHAL_CP3_SA_ALIGN)))
1 attribute((aligned(XCHAL_CP2_SA_ALIGN)))
1 attribute((aligned(XCHAL_CP1_SA_ALIGN)))
1 attribute((aligned(XCHAL_CP0_SA_ALIGN)))
1 attribute((aligned(VRING_DESC_ALIGN_SIZE)))
1 attribute((aligned(VRING_AVAIL_ALIGN_SIZE)))
1 attribute((aligned(TSB_ENTRY_ALIGNMENT)))
1 attribute((aligned(TPACKET_ALIGN(x))))
1 attribute((aligned(TPACKET_ALIGNMENT)))
1 attribute((aligned(stackalign)))
1 attribute((aligned(sizeof(u64))))
1 attribute((aligned(sizeof(u32))))
1 attribute((aligned(sizeof(__u32))))
1 attribute((aligned(sizeof(s64))))
1 attribute((aligned(sizeof(kernel_ulong_t))))
1 attribute((aligned(size)))
1 attribute((aligned(PS3_BMP_MINALIGN)))
1 attribute((aligned(_K_SS_ALIGNSIZE)))
1 attribute((aligned(INTERNODE_CACHE_BYTES)))
1 attribute((aligned(GRU_CACHE_LINE_BYTES)))
1 attribute((aligned(DM_IO_MAX_REGIONS)))
1 attribute((aligned(CVMX_CACHE_LINE_SIZE)))
1 attribute((aligned(CRYPTO_MINALIGN)))
1 attribute((aligned(CPPI_DESCRIPTOR_ALIGN)))
1 attribute((aligned(ALIGN_SIZE)))
1 attribute((aligned(alignof(u32))))
1 attribute((aligned(alignof(structnft_expr))))
1 attribute((aligned(alignof(structhash_alg_common))))
1 attribute((aligned(4096)))
1 attribute((aligned(2*sizeof(long))))
1 attribute((aligned(2),packed))
1 attribute((aligned(256),packed))
1 attribute((aligned(2048)))
1 attribute((aligned(2)))
1 attribute((aligned(1<<(INTERNODE_CACHE_SHIFT))))
1 attribute((aligned(1024),packed))
1 attribute((aligned(0x80)))
1 attribute((aligned(0x2000)))
1 attribute((aligned))
1 attribute((alias("vtime_account_irq_enter")))
1 attribute((alias("thermal_genl_event_tz_trip_up")))
1 attribute((alias("thermal_genl_event_tz_trip_add")))
1 attribute((alias(#system"_mv")))
1 attribute((alias(#symbol)))
1 attribute((alias(__stringify(__s390x_sys_##sname))))
1 attribute((alias("octeon_board_type_string")))
1 attribute((alias("native_sched_clock")))
1 attribute((alias(istringify(SIG_EXPR_LIST_SYM(sig,group)))))
1 attribute((alias(#internal_name)))
1 attribute((alias(#initfn)))
1 attribute((alias(#exitfn)))
1 attribute((address_space(257))
1 attribute((address_space(256))
1 attribute((address_space(0)))
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 12:08 ` Joe Perches
@ 2020-10-23 19:14 ` Dwaipayan Ray
2020-10-23 20:42 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Dwaipayan Ray @ 2020-10-23 19:14 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
>>> And you could check using
>>>
>>> $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*)/
>>>
>>> and check for all attributes in $1 after
>>> stripping the leading and trailing parens
>>> and any leading and trailing underscores
>>> from each attribute.
>>>
>>> And you only need one hash and you should
>>> check for existence of the key rather than
>>> have multiple lists.
>>>
>>> if (exists($attributes($attr))) {
>>>
>> Okay thanks!
>> But what should be done for the attributes which are
>> parameterized, like __aligned__(x). Should all the __
>> for these entries be trimmed too?
> yes
>
>> There are also
>> cases where there are multiple versions like:
>>
>> __aligned__
>> __aligned__(x)
> $ git grep __aligned__ | grep -v -P '__aligned__\s*\('
>
> AFAIK: There is only one use of bare __aligned__ and
> that's in compiler_attributes.h
>
>> To help differentiate between them what can be done?
>> Should i make the keys as:
>>
>> aligned
>> aligned__(
>>
>> instead of
>>
>> __aligned__
>> __aligned__(
> Just use aligned
>
> Just fyi:
>
> these are the uses of __attribute__ in the kernel
> with all the underscores and spaces removed so there's
> some value in finding the multiple actual attributes .
Hi,
I modified the check to check the attributes from the map.
There are two checks - one for the normal attributes and
one for the ones with arguments, which needs just a bit more processing.
So attributes like __packed__ as well as those like
__aligned__(x) are handled.
What do you think?
---
+ $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*\)/) {
+ my $attr = trim($1);
+ $attr =~ s/\(\s*_*(.*)\)/$1/;
+ while($attr =~ s/(.*)_$/$1/) {} # Remove trailing underscores
+
+ 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 (exists($attr_list{$attr})) {
+ my $new = $attr_list{$attr};
+ WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+ "$new is preffered over
__attribute__((__${attr}__))\n" . $herecurr);
+ }
+
+ # Check for attributes with parameters, like copy__(symbol)
+ if ($attr =~ /(.*)__(\(.*\))/) {
+ if (exists($attr_list{$1})) {
+ my $new = $attr_list{$1};
+ WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+ "$new$2 is preffered over
__attribute__((__${attr}))\n" . $herecurr);
+ }
+ }
---
If this is okay I would like to send in a proper v3.
Thanks,
Dwaipayan.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 19:14 ` Dwaipayan Ray
@ 2020-10-23 20:42 ` Joe Perches
2020-10-23 20:57 ` Dwaipayan Ray
0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2020-10-23 20:42 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
On Sat, 2020-10-24 at 00:44 +0530, Dwaipayan Ray wrote:
> Hi,
Hi again.
> I modified the check to check the attributes from the map.
> There are two checks - one for the normal attributes and
> one for the ones with arguments, which needs just a bit more
> processing.
>
> So attributes like __packed__ as well as those like
> __aligned__(x) are handled.
>
> What do you think?
>
> ---
> + $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*\)/)
> {
> + my $attr = trim($1);
> + $attr =~ s/\(\s*_*(.*)\)/$1/;
> + while($attr =~ s/(.*)_$/$1/) {} # Remove trailing
> underscores
I think this could be a single test like:
while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) {
my $curr_attr = $1;
my $parens = $2;
$curr_attr = s/^_*(.*)_*$/$1/;
> + my %attr_list = (
> + "alias" => "__alias",
> + "aligned" => "__aligned",
These might be better using tab alignment.
And you might special case format(printf/scanf here too
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 20:42 ` Joe Perches
@ 2020-10-23 20:57 ` Dwaipayan Ray
2020-10-23 21:04 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Dwaipayan Ray @ 2020-10-23 20:57 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
On Sat, Oct 24, 2020 at 2:12 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-10-24 at 00:44 +0530, Dwaipayan Ray wrote:
> > Hi,
>
> Hi again.
>
> > I modified the check to check the attributes from the map.
> > There are two checks - one for the normal attributes and
> > one for the ones with arguments, which needs just a bit more
> > processing.
> >
> > So attributes like __packed__ as well as those like
> > __aligned__(x) are handled.
> >
> > What do you think?
> >
> > ---
> > + $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*\)/)
> > {
> > + my $attr = trim($1);
> > + $attr =~ s/\(\s*_*(.*)\)/$1/;
> > + while($attr =~ s/(.*)_$/$1/) {} # Remove trailing
> > underscores
>
> I think this could be a single test like:
>
> while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) {
> my $curr_attr = $1;
> my $parens = $2;
> $curr_attr = s/^_*(.*)_*$/$1/;
>
Thanks, I will do that.
Also I tried the pattern attr =~ s/^_*(.*)_*$/$1/
for trimming the _ earlier. I think it doesn't trim the
trailing underscores in the suffix as (.*) captures everything greedily.
Is the iterative one perhaps okay instead?
while($attr =~ s/(.*)_$/$1/) {}
> > + my %attr_list = (
> > + "alias" => "__alias",
> > + "aligned" => "__aligned",
>
> These might be better using tab alignment.
>
> And you might special case format(printf/scanf here too
Yes I think they are special cases now too. I didn't post the
entire context, so my mistake.
Thanks,
Dwaipayan.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 20:57 ` Dwaipayan Ray
@ 2020-10-23 21:04 ` Joe Perches
2020-10-23 21:09 ` Dwaipayan Ray
0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2020-10-23 21:04 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
On Sat, 2020-10-24 at 02:27 +0530, Dwaipayan Ray wrote:
> Also I tried the pattern attr =~ s/^_*(.*)_*$/$1/
> for trimming the _ earlier. I think it doesn't trim the
> trailing underscores in the suffix as (.*) captures everything greedily.
>
> Is the iterative one perhaps okay instead?
> while($attr =~ s/(.*)_$/$1/) {}
Then perhaps
$curr_attr =~ s/^_+//; $curr_attr =~ s/_+$//;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns
2020-10-23 21:04 ` Joe Perches
@ 2020-10-23 21:09 ` Dwaipayan Ray
0 siblings, 0 replies; 9+ messages in thread
From: Dwaipayan Ray @ 2020-10-23 21:09 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, Lukas Bulwahn
On Sat, Oct 24, 2020 at 2:34 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-10-24 at 02:27 +0530, Dwaipayan Ray wrote:
> > Also I tried the pattern attr =~ s/^_*(.*)_*$/$1/
> > for trimming the _ earlier. I think it doesn't trim the
> > trailing underscores in the suffix as (.*) captures everything greedily.
> >
> > Is the iterative one perhaps okay instead?
> > while($attr =~ s/(.*)_$/$1/) {}
>
> Then perhaps
>
> $curr_attr =~ s/^_+//; $curr_attr =~ s/_+$//;
>
Okay thanks!
I will send in a new version when everything looks good
and tested.
Regards,
Dwaipayan.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-23 21:10 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 9:43 [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns Dwaipayan Ray
2020-10-23 11:04 ` Joe Perches
2020-10-23 11:40 ` Dwaipayan Ray
2020-10-23 12:08 ` Joe Perches
2020-10-23 19:14 ` Dwaipayan Ray
2020-10-23 20:42 ` Joe Perches
2020-10-23 20:57 ` Dwaipayan Ray
2020-10-23 21:04 ` Joe Perches
2020-10-23 21:09 ` Dwaipayan Ray
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).