[Linux-kernel-mentees] [PATCH RFC] checkpatch: extend attributes check to handle more patterns

Lukas Bulwahn lukas.bulwahn at gmail.com
Thu Oct 22 14:20:19 UTC 2020



On Thu, 22 Oct 2020, 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.
>

Did you checkpatch this patch?

I always thought checkpatch warns to use ./include/...
as path when somebody just use include/... in a commit message.

 
> Checkpatch currently checks __attribute__ for each of
> packed, aligned, printf, scanf, and weak. Other declarations
> in compiler_attributes.h are not handled.
> 
> Add more definitions to the attribute check.
> The following patterns are added:
> 
> __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/
> Signed-off-by: Dwaipayan Ray <dwaipayanray1 at gmail.com>
> ---


I guess you could add suggested-by.

I am wondering if this could be refactored into a shared function and if 
all different warning messages each deserve to be an own type.

But send it out as-is to Joe and the list and we continue the discussion 
there.


Lukas

>  scripts/checkpatch.pl | 199 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 163 insertions(+), 36 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7e505688257a..869d9b2801cd 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6155,50 +6155,177 @@ 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);
>  
> -# 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__ alias, prefer __alias
> +			if ($attr =~/^_*alias_*/) {
> +				WARN("PREFER_ALIAS",
> +				     "__alias(symbol) is preferred over __attribute__((alias(#symbol)))\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__ aligned(size), prefer __aligned(size)
> +			if ($attr =~/^_*aligned_*\(/) {
> +				WARN("PREFER_ALIGNED",
> +				     "__aligned(size) is preferred over __attribute__((__aligned__(size)))\n" . $herecurr);
>  			}
> -		}
>  
> -# 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__ aligned, prefer __aligned_largest
> +			if ($attr =~/^_*aligned_*$/) {
> +				WARN("PREFER_ALIGNED_LARGEST",
> +				     "__aligned_largest is preferred over __attribute__((__aligned__))\n" . $herecurr);
> +			}
>  
> +			# Check for __attribute__ always_inline, prefer __always_inline
> +			if ($attr =~/^_*always_inline_*/) {
> +				WARN("PREFER_ALWAYS_INLINE",
> +				     "__always_inline is preferred over __attribute__((__always_inline__))\n" . $herecurr);
>  			}
> -		}
>  
> -# 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__ assume_aligned, prefer __assume_aligned
> +			if ($attr =~/^_*assume_aligned_*/) {
> +				WARN("PREFER_ASSUME_ALIGNED",
> +				     "__asume_aligned(a, ...) is preferred over __attribute__((__assume_aligned__(a, ## __VA_ARGS__)))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ cold, prefer __cold
> +			if ($attr =~ /^_*cold_*/) {
> +				WARN("PREFER_COLD",
> +				     "__cold is preferred over __attribute__((__cold__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ const, prefer __attribute_const
> +			if ($attr =~ /^_*const_*/) {
> +				WARN("PREFER_CONST",
> +				     "__attribute_const is preferred over __attribute__((__const__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ copy, prefer __copy
> +			if ($attr =~ /^_*copy_*/) {
> +				WARN("PREFER_COPY",
> +				     "__copy(symbol) is preferred over __attribute__((__copy__(symbol)))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ desginated_init, prefer __designated_init
> +			if ($attr =~ /^_*designated_init_*/) {
> +				WARN("PREFER_DESIGNATED_INIT",
> +				     "__designated_init is preferred over __attribute__((__designated_init__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ externally_visible, prefer __visible
> +			if ($attr =~ /^_*externally_visible_*/) {
> +				WARN("PREFER_VISIBLE",
> +				     "__visible is preferred over __attribute__((__externally_visible__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ format(printf, prefer __printf
> +			if ($attr =~ /^_*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_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__ gnu_inline, prefer __gnu_inline
> +			if ($attr =~ /^_*gnu_inline_*/) {
> +				WARN("PREFER_GNU_INLINE",
> +				     "__gnu_inline is preferred over __attribute__((__gnu_inline__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ malloc, prefer __malloc
> +			if ($attr =~ /^_*malloc_*/) {
> +				WARN("PREFER_MALLOC",
> +				     "__malloc is preferred over __attribute__((__malloc__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ mode, prefer __mode
> +			if ($attr =~ /^_*mode_*/) {
> +				WARN("PREFER_MODE",
> +				     "__mode(x) is preferred over __attribute__((__mode__(x)))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ no_caller_saved_registers, prefer __no_caller_saved_registers
> +			if ($attr =~ /^_*no_caller_saved_registers_*/) {
> +				WARN("PREFER_NO_CALLER_SAVED_REGISTERS",
> +				     "__no_caller_saved_registers is preferred over __attribute__((__no_caller_saved_registers__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ noclone, prefer __noclone
> +			if ($attr =~ /^_*noclone_*/) {
> +				WARN("PREFER_NOCLONE",
> +				     "__noclone is preferred over __attribute__((__noclone__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ fallthrough, prefer fallthrough
> +			if ($attr =~ /^_*fallthrough_*/) {
> +				WARN("PREFER_FALLTHROUGH",
> +				     "fallthrough is preferred over __attribute__((__fallthrough__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ noinline, prefer noinline
> +			if ($attr =~ /^_*noinline_*/) {
> +				WARN("PREFER_NOINLINE",
> +				     "noinline is preferred over __attribute__((__noinline__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ nonstring, prefer __nonstring
> +			if ($attr =~ /^_*nonstring_*/) {
> +				WARN("PREFER_NONSTRING",
> +				     "__nonstring is preferred over __attribute__((__nonstring__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ noreturn, prefer __noreturn
> +			if ($attr =~ /^_*noreturn_*/) {
> +				WARN("PREFER_NORETURN",
> +				     "__noreturn is preferred over __attribute__((__noreturn__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ packed, prefer __packed
> +			if ($attr =~ /^_*packed_*/) {
> +				WARN("PREFER_PACKED",
> +				     "__packed is preferred over __attribute__((__packed__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ pure, prefer __pure
> +			if ($attr =~ /^_*pure_*/) {
> +				WARN("PREFER_PURE",
> +				     "__pure is preferred over __attribute__((__pure__))\n" . $herecurr);
> +			}
> +
> +			# 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_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__ unused, prefer __always_unused or __maybe_unused
> +			if ($attr =~ /^_*unused_*/) {
> +				WARN("PREFER_UNUSED",
> +				     "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr);
> +			}
> +
> +			# Check for __attribute__ used, prefer __used
> +			if ($attr =~ /^_*used_*/) {
> +				WARN("PREFER_USED",
> +				     "__used is preferred over __attribute__((__used__))\n" . $herecurr);
>  			}
>  		}
>  
> -- 
> 2.27.0
> 
> 


More information about the Linux-kernel-mentees mailing list