Re: [PATCH] checkpatch: types found in one source file do not affect processing of others

[Alex Dowad <alexinbeijing@gmail.com>]

On 18/05/15 20:42, Joe Perches wrote:
> On Mon, 2015-05-18 at 15:33 +0200, Alex Dowad wrote:
>> checkpatch uses various cues in its input files to discover the names of
>> user-defined types. It then uses that information when processing expressions,
>> to discover more style issues.
>>
>> Unfortunately, in rare cases, this means that checkpatch may give different
>> results if you run it on several files at the same time, 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.
>>
>> As an example, drivers/staging/media/bcm2048/radio-bcm2048.c contains this
>> line (in a macro):
>>
>>    size value;
>>
>> Then drivers/staging/media/davinci_vpfe/vpfe_video.c has this line:
>>
>>    while (size * *nbuffers > vpfe_dev->video_limit)
>>
>> If checkpatch processes these 2 files together, 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!
>>
>> Therefore, keep user-defined types in a separate array from built-in ones,
>> and reset the array of user-defined types at the beginning of each new
>> source file.
>>
> I suggest this:
> ---
>   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 {
>
>
Looks good! AD