checkpatch: types found in one source file do not affect processing of others

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.

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 am not a Perl programmer -- please let me know if there is a better way to accomplish what I am trying to do here.

scripts/checkpatch.pl +1 -1

@@ -511,7 +515,7 @@ $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 $all = "(?x:  \n" . join("|\n  ", @allTypeList) . "\n)";
 	my $Misordered = "(?x:  \n" . join("|\n  ", @typeListMisordered) . "\n)";
 	my $allWithAttr = "(?x:  \n" . join("|\n  ", @typeListWithAttr) . "\n)";
 	$Modifier	= qr{(?:$Attribute|$Sparse|$mods)};

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.

Signed-off-by Alex Dowad <[email protected]>

---

Dear patch checkers,

I am not a Perl programmer -- please let me know if there is a better way to accomplish what I am trying to do here.

Your feedback will be appreciated,
Alex Dowad

 scripts/checkpatch.pl | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

scripts/checkpatch.pl +0 -0

index 89b1df4..5a5668f 100755

scripts/checkpatch.pl +2 -1

 our @modifierList = (
 	qr{fastcall},
 );

@@ -511,7 +515,7 @@ $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 $all = "(?x:  \n" . join("|\n  ", @allTypeList) . "\n)";
 	my $Misordered = "(?x:  \n" . join("|\n  ", @typeListMisordered) . "\n)";
 	my $allWithAttr = "(?x:  \n" . join("|\n  ", @typeListWithAttr) . "\n)";
 	$Modifier	= qr{(?:$Attribute|$Sparse|$mods)};

@@ -745,6 +749,7 @@ for my $filename (@ARGV) {
 	@fixed = ();
 	@fixed_inserted = ();
 	@fixed_deleted = ();
+	@allTypeList = (@typeList);
 	$fixlinenr = -1;
 }
 

2.0.0.GIT

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.

Signed-off-by Alex Dowad <[email protected]>

---

Dear patch checkers,

I am not a Perl programmer -- please let me know if there is a better way to accomplish what I am trying to do here.

Your feedback will be appreciated,
Alex Dowad

 scripts/checkpatch.pl | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Quoted 19 lines · Alex Dowad

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)

Quoted 8 lines

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(-)

scripts/checkpatch.pl +8 -3

index 89b1df4..174d711 100755
@@ -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);

From: Alex Dowad <[email protected]>

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 <[email protected]>

Signed-off-by Joe Perches <[email protected]>

---

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(-)

scripts/checkpatch.pl +8 -3

index 89b1df4..174d711 100755
@@ -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);