On Mon, May 18, 2015 at 05:22:25PM -0700, Joe Perches wrote:
> From: Alex Dowad <alexinbeij...@gmail.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 <alexinbeij...@gmail.com>
> Signed-off-by: Joe Perches <j...@perches.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 <a...@canonical.com>
-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
