Messages in this thread | | | Date | Mon, 18 May 2015 21:28:08 +0200 | From | Alex Dowad <> | Subject | Re: [PATCH] checkpatch: types found in one source file do not affect processing of others |
| |
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
| |