linux-kernel - [Acked] [PATCH V2] checkpatch: Make types found in a source file/patch local

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] [day] [month] [year] [list]

Message-ID: <20150519103456.GH3751@dm>
Date:	Tue, 19 May 2015 11:34:56 +0100
From:	Andy Whitcroft <apw@...onical.com>
To:	Joe Perches <joe@...ches.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Alex Dowad <alexinbeijing@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: [Acked] [PATCH V2] checkpatch: Make types found in a source
 file/patch local

On Mon, May 18, 2015 at 05:22:25PM -0700, Joe Perches wrote:
> From: Alex Dowad <alexinbeijing@...il.com>
> 
> checkpatch uses various cues in its input patches and files to discover the
> names of user-defined types and modifiers. It then uses that information when
> processing expressions to discover potential style issues.
> 
> Unfortunately, in rare cases, this means that checkpatch may give different
> results if you run it on several input files in one execution, or one by one!
> 
> The reason is that it may identify a type (or something that looks like a type)
> in one file, and then carry this information over when processing a different
> file.
> 
> For example, drivers/staging/media/bcm2048/radio-bcm2048.c contains this
> line (in a macro):
> 
> 	size value;
> 
> and drivers/staging/media/davinci_vpfe/vpfe_video.c has this line:
> 
> 	while (size * *nbuffers > vpfe_dev->video_limit)
> 
> If checkpatch processes these 2 files in a single command like:
> 	./scripts/checkpatch.pl -f $file1 $file2
> the (spurious) "size" type detected in the first file will cause it to flag
> the second file for improper use of the pointer dereference operator.
> 
> To fix this, store types and modifiers found in a file in separate arrays
> from built-in ones, and reset the arrays of types and modifiers found
> in files at the beginning of each new source file.
> 
> Signed-off-by: Alex Dowad <alexinbeijing@...il.com>
> Signed-off-by: Joe Perches <joe@...ches.com>
> ---
> 
> I kept Alex's Signed-off-line as he found the issue, wrote the original
> changelog and patch.  I did some perl neatening and added the modifier block.
> 
>  scripts/checkpatch.pl | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 89b1df4..174d711 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -418,6 +418,7 @@ our @typeList = (
>  	qr{${Ident}_handler_fn},
>  	@typeListMisordered,
>  );
> +our @typeListFile = ();
>  our @typeListWithAttr = (
>  	@typeList,
>  	qr{struct\s+$InitAttribute\s+$Ident},
> @@ -427,6 +428,7 @@ our @typeListWithAttr = (
>  our @modifierList = (
>  	qr{fastcall},
>  );
> +our @modifierListFile = ();
>  
>  our @mode_permission_funcs = (
>  	["module_param", 3],
> @@ -510,8 +512,8 @@ if ($codespell) {
>  $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix;
>  
>  sub build_types {
> -	my $mods = "(?x:  \n" . join("|\n  ", @modifierList) . "\n)";
> -	my $all = "(?x:  \n" . join("|\n  ", @typeList) . "\n)";
> +	my $mods = "(?x:  \n" . join("|\n  ", (@modifierList, @modifierListFile)) . "\n)";
> +	my $all = "(?x:  \n" . join("|\n  ", (@typeList, @typeListFile)) . "\n)";
>  	my $Misordered = "(?x:  \n" . join("|\n  ", @typeListMisordered) . "\n)";
>  	my $allWithAttr = "(?x:  \n" . join("|\n  ", @typeListWithAttr) . "\n)";
>  	$Modifier	= qr{(?:$Attribute|$Sparse|$mods)};
> @@ -746,6 +748,9 @@ for my $filename (@ARGV) {
>  	@fixed_inserted = ();
>  	@fixed_deleted = ();
>  	$fixlinenr = -1;
> +	@modifierListFile = ();
> +	@typeListFile = ();
> +	build_types();
>  }
>  
>  exit($exit);
> @@ -1610,13 +1615,13 @@ sub possible {
>  			for my $modifier (split(' ', $possible)) {
>  				if ($modifier !~ $notPermitted) {
>  					warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible);
> -					push(@modifierList, $modifier);
> +					push(@modifierListFile, $modifier);
>  				}
>  			}
>  
>  		} else {
>  			warn "POSSIBLE: $possible ($line)\n" if ($dbg_possible);
> -			push(@typeList, $possible);
> +			push(@typeListFile, $possible);
>  		}
>  		build_types();
>  	} else {

That looks about right to me.

Acked-by: Andy Whitcroft <apw@...onical.com>

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/