* [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 [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
Powered by JetHome