linux-kernel - Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d2b05b45adbcf3f1d16692b054862a7aa7353f6d.camel@perches.com>
Date:   Fri, 23 Oct 2020 04:04:42 -0700
From:   Joe Perches <joe@...ches.com>
To:     Dwaipayan Ray <dwaipayanray1@...il.com>
Cc:     linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel@...r.kernel.org, lukas.bulwahn@...il.com
Subject: Re: [PATCH RFC v2] checkpatch: extend attributes check to handle
 more patterns

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);
> +				}
> +			}