linux-kernel - Re: [PATCH v5] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <52f88d0bb938c5063ede693a05bf64961af5a496.camel@perches.com>
Date:   Sun, 25 Oct 2020 10:59:09 -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 v5] checkpatch: extend attributes check to handle more
 patterns

On Sun, 2020-10-25 at 15:45 +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, section, 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.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -6155,50 +6155,95 @@ 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);
> -		}
> +		    $rawline =~ /\b__attribute__\s*\(\s*($balanced_parens)\s*\)/) {

Using $rawline would also change comments and that seems wrong.
Any reason to use $rawline instead of $line?

[]

> +			if (scalar @conv_array > 0 && $conv_possible == 1) {
> +				my $replace = join(' ', @conv_array);
> +				if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> +				         "$replace is preferred over __attribute__(($attr))\n" . $herecurr) &&
> +					$fix) {
> +					$fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*\Q$attr\E\s*\)\s*\)/$replace/;

I looks it would be useful to add
					$fixed[$fixlinenr] =~ s/\}\Q$replace\E/} $replace/;
so there's a space added between } and any replacements.