diff mbox

checkpatch: add --typedefsfile

Message ID ba1124d6dfa599bb0dd1d8919dd45dd09ce541a4.1492702192.git.jerome.forissier@linaro.org
State Accepted
Commit 75ad8c575a5ad105e2afc2051c68abceb9c65431
Headers show

Commit Message

Jerome Forissier April 20, 2017, 3:39 p.m. UTC 
When using checkpatch on out-of-tree code, it may occur that some
project-specific types are used, which will cause spurious warnings.
Add the --typedefsfile option as a way to extend the known types and
deal with this issue.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

---
 scripts/checkpatch.pl | 56 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 19 deletions(-)

-- 
2.7.4

Comments

Joe Perches April 20, 2017, 4:49 p.m. UTC | #1 
On Thu, 2017-04-20 at 17:39 +0200, Jerome Forissier wrote:
> When using checkpatch on out-of-tree code, it may occur that some

> project-specific types are used, which will cause spurious warnings.

> Add the --typedefsfile option as a way to extend the known types and

> deal with this issue.


I'm not opposed to the addition.
What out-of-tree project is this for?

> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

> ---

>  scripts/checkpatch.pl | 56 ++++++++++++++++++++++++++++++++++-----------------

>  1 file changed, 37 insertions(+), 19 deletions(-)

> 

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl

> index baa3c7b..eb55f5f 100755

> --- a/scripts/checkpatch.pl

> +++ b/scripts/checkpatch.pl

> @@ -55,6 +55,7 @@ my $spelling_file = "$D/spelling.txt";

>  my $codespell = 0;

>  my $codespellfile = "/usr/share/codespell/dictionary.txt";

>  my $conststructsfile = "$D/const_structs.checkpatch";

> +my $typedefsfile = "";

>  my $color = 1;

>  my $allow_c99_comments = 1;

>  

> @@ -113,6 +114,7 @@ Options:

>    --codespell                Use the codespell dictionary for spelling/typos

>                               (default:/usr/share/codespell/dictionary.txt)

>    --codespellfile            Use this codespell dictionary

> +  --typedefsfile             Read additional types from this file

>    --color                    Use colors when output is STDOUT (default: on)

>    -h, --help, --version      display this help and exit

>  

> @@ -208,6 +210,7 @@ GetOptions(

>  	'test-only=s'	=> \$tst_only,

>  	'codespell!'	=> \$codespell,

>  	'codespellfile=s'	=> \$codespellfile,

> +	'typedefsfile=s'	=> \$typedefsfile,

>  	'color!'	=> \$color,

>  	'h|help'	=> \$help,

>  	'version'	=> \$help

> @@ -629,28 +632,43 @@ if ($codespell) {

>  

>  $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix;

>  

> +sub read_words {

> +	my ($wordsRef, $file) = @_;

> +

> +	if (open(my $words, '<', $file)) {

> +		while (<$words>) {

> +			my $line = $_;

> +

> +			$line =~ s/\s*\n?$//g;

> +			$line =~ s/^\s*//g;

> +

> +			next if ($line =~ m/^\s*#/);

> +			next if ($line =~ m/^\s*$/);

> +			if ($line =~ /\s/) {

> +				print("$file: '$line' invalid - ignored\n");

> +				next;

> +			}

> +

> +			$$wordsRef .= '|' if ($$wordsRef ne "");

> +			$$wordsRef .= $line;

> +		}

> +		close($file);

> +		return 1;

> +	}

> +

> +	return 0;

> +}

> +

>  my $const_structs = "";

> -if (open(my $conststructs, '<', $conststructsfile)) {

> -	while (<$conststructs>) {

> -		my $line = $_;

> +read_words(\$const_structs, $conststructsfile)

> +    or warn "No structs that should be const will be found - file '$conststructsfile': $!\n";

>  

> -		$line =~ s/\s*\n?$//g;

> -		$line =~ s/^\s*//g;

> -

> -		next if ($line =~ m/^\s*#/);

> -		next if ($line =~ m/^\s*$/);

> -		if ($line =~ /\s/) {

> -			print("$conststructsfile: '$line' invalid - ignored\n");

> -			next;

> -		}

> -

> -		$const_structs .= '|' if ($const_structs ne "");

> -		$const_structs .= $line;

> -	}

> -	close($conststructsfile);

> -} else {

> -	warn "No structs that should be const will be found - file '$conststructsfile': $!\n";

> +my $typeOtherTypedefs = "";

> +if (length($typedefsfile)) {

> +	read_words(\$typeOtherTypedefs, $typedefsfile)

> +	    or warn "No additional types will be considered - file '$typedefsfile': $!\n";

>  }

> +$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne "");

>  

>  sub build_types {

>  	my $mods = "(?x:  \n" . join("|\n  ", (@modifierList, @modifierListFile)) . "\n)";
Jerome Forissier April 21, 2017, 6:31 a.m. UTC | #2 
On 04/20/2017 06:49 PM, Joe Perches wrote:
> On Thu, 2017-04-20 at 17:39 +0200, Jerome Forissier wrote:

>> When using checkpatch on out-of-tree code, it may occur that some

>> project-specific types are used, which will cause spurious warnings.

>> Add the --typedefsfile option as a way to extend the known types and

>> deal with this issue.

> 

> I'm not opposed to the addition.

> What out-of-tree project is this for?


OP-TEE [1]. We run a Travis job on all pull requests [2], and checkpatch
is part of that. The typical false warning we get on a regular basis is
with some pointers to functions returning TEE_Result [3], which is a
typedef from the GlobalPlatform APIs. We consider it is acceptable to
use GP types in the OP-TEE core implementation, that's why this patch
would be helpful for us.

[1] https://github.com/OP-TEE/optee_os
[2] https://travis-ci.org/OP-TEE/optee_os/builds
[3] https://travis-ci.org/OP-TEE/optee_os/builds/193355335#L1733

Thanks,
-- 
Jerome

> 

>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

>> ---

>>  scripts/checkpatch.pl | 56 ++++++++++++++++++++++++++++++++++-----------------

>>  1 file changed, 37 insertions(+), 19 deletions(-)

>>

>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl

>> index baa3c7b..eb55f5f 100755

>> --- a/scripts/checkpatch.pl

>> +++ b/scripts/checkpatch.pl

>> @@ -55,6 +55,7 @@ my $spelling_file = "$D/spelling.txt";

>>  my $codespell = 0;

>>  my $codespellfile = "/usr/share/codespell/dictionary.txt";

>>  my $conststructsfile = "$D/const_structs.checkpatch";

>> +my $typedefsfile = "";

>>  my $color = 1;

>>  my $allow_c99_comments = 1;

>>  

>> @@ -113,6 +114,7 @@ Options:

>>    --codespell                Use the codespell dictionary for spelling/typos

>>                               (default:/usr/share/codespell/dictionary.txt)

>>    --codespellfile            Use this codespell dictionary

>> +  --typedefsfile             Read additional types from this file

>>    --color                    Use colors when output is STDOUT (default: on)

>>    -h, --help, --version      display this help and exit

>>  

>> @@ -208,6 +210,7 @@ GetOptions(

>>  	'test-only=s'	=> \$tst_only,

>>  	'codespell!'	=> \$codespell,

>>  	'codespellfile=s'	=> \$codespellfile,

>> +	'typedefsfile=s'	=> \$typedefsfile,

>>  	'color!'	=> \$color,

>>  	'h|help'	=> \$help,

>>  	'version'	=> \$help

>> @@ -629,28 +632,43 @@ if ($codespell) {

>>  

>>  $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix;

>>  

>> +sub read_words {

>> +	my ($wordsRef, $file) = @_;

>> +

>> +	if (open(my $words, '<', $file)) {

>> +		while (<$words>) {

>> +			my $line = $_;

>> +

>> +			$line =~ s/\s*\n?$//g;

>> +			$line =~ s/^\s*//g;

>> +

>> +			next if ($line =~ m/^\s*#/);

>> +			next if ($line =~ m/^\s*$/);

>> +			if ($line =~ /\s/) {

>> +				print("$file: '$line' invalid - ignored\n");

>> +				next;

>> +			}

>> +

>> +			$$wordsRef .= '|' if ($$wordsRef ne "");

>> +			$$wordsRef .= $line;

>> +		}

>> +		close($file);

>> +		return 1;

>> +	}

>> +

>> +	return 0;

>> +}

>> +

>>  my $const_structs = "";

>> -if (open(my $conststructs, '<', $conststructsfile)) {

>> -	while (<$conststructs>) {

>> -		my $line = $_;

>> +read_words(\$const_structs, $conststructsfile)

>> +    or warn "No structs that should be const will be found - file '$conststructsfile': $!\n";

>>  

>> -		$line =~ s/\s*\n?$//g;

>> -		$line =~ s/^\s*//g;

>> -

>> -		next if ($line =~ m/^\s*#/);

>> -		next if ($line =~ m/^\s*$/);

>> -		if ($line =~ /\s/) {

>> -			print("$conststructsfile: '$line' invalid - ignored\n");

>> -			next;

>> -		}

>> -

>> -		$const_structs .= '|' if ($const_structs ne "");

>> -		$const_structs .= $line;

>> -	}

>> -	close($conststructsfile);

>> -} else {

>> -	warn "No structs that should be const will be found - file '$conststructsfile': $!\n";

>> +my $typeOtherTypedefs = "";

>> +if (length($typedefsfile)) {

>> +	read_words(\$typeOtherTypedefs, $typedefsfile)

>> +	    or warn "No additional types will be considered - file '$typedefsfile': $!\n";

>>  }

>> +$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne "");

>>  

>>  sub build_types {

>>  	my $mods = "(?x:  \n" . join("|\n  ", (@modifierList, @modifierListFile)) . "\n)";
Jerome Forissier April 27, 2017, 3:41 p.m. UTC | #3 
On 04/21/2017 08:31 AM, Jerome Forissier wrote:
> On 04/20/2017 06:49 PM, Joe Perches wrote:

>> On Thu, 2017-04-20 at 17:39 +0200, Jerome Forissier wrote:

>>> When using checkpatch on out-of-tree code, it may occur that some

>>> project-specific types are used, which will cause spurious warnings.

>>> Add the --typedefsfile option as a way to extend the known types and

>>> deal with this issue.

>>

>> I'm not opposed to the addition.

>> What out-of-tree project is this for?

> 

> OP-TEE [1]. We run a Travis job on all pull requests [2], and checkpatch

> is part of that. The typical false warning we get on a regular basis is

> with some pointers to functions returning TEE_Result [3], which is a

> typedef from the GlobalPlatform APIs. We consider it is acceptable to

> use GP types in the OP-TEE core implementation, that's why this patch

> would be helpful for us.

> 

> [1] https://github.com/OP-TEE/optee_os

> [2] https://travis-ci.org/OP-TEE/optee_os/builds

> [3] https://travis-ci.org/OP-TEE/optee_os/builds/193355335#L1733 


Ping?
Joe Perches April 27, 2017, 3:59 p.m. UTC | #4 
On Thu, 2017-04-27 at 17:41 +0200, Jerome Forissier wrote:
> On 04/21/2017 08:31 AM, Jerome Forissier wrote:

> > On 04/20/2017 06:49 PM, Joe Perches wrote:

> > > On Thu, 2017-04-20 at 17:39 +0200, Jerome Forissier wrote:

> > > > When using checkpatch on out-of-tree code, it may occur that some

> > > > project-specific types are used, which will cause spurious warnings.

> > > > Add the --typedefsfile option as a way to extend the known types and

> > > > deal with this issue.

> > > 

> > > I'm not opposed to the addition.

> > > What out-of-tree project is this for?

> > 

> > OP-TEE [1]. We run a Travis job on all pull requests [2], and checkpatch

> > is part of that. The typical false warning we get on a regular basis is

> > with some pointers to functions returning TEE_Result [3], which is a

> > typedef from the GlobalPlatform APIs. We consider it is acceptable to

> > use GP types in the OP-TEE core implementation, that's why this patch

> > would be helpful for us.

> > 

> > [1] https://github.com/OP-TEE/optee_os

> > [2] https://travis-ci.org/OP-TEE/optee_os/builds

> > [3] https://travis-ci.org/OP-TEE/optee_os/builds/193355335#L1733 

> 

> Ping?


It's a well written patch.

But I'll leave it up to Andrew Morton to accept/reject this.

I'm not opposed to it though as it seems reasonable because
using a checkpatch command-line --ignore=NEW_TYPEDEFS may
not be the right solution for your use case.
diff mbox

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index baa3c7b..eb55f5f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -55,6 +55,7 @@  my $spelling_file = "$D/spelling.txt";
 my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
+my $typedefsfile = "";
 my $color = 1;
 my $allow_c99_comments = 1;
 
@@ -113,6 +114,7 @@  Options:
   --codespell                Use the codespell dictionary for spelling/typos
                              (default:/usr/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
+  --typedefsfile             Read additional types from this file
   --color                    Use colors when output is STDOUT (default: on)
   -h, --help, --version      display this help and exit
 
@@ -208,6 +210,7 @@  GetOptions(
 	'test-only=s'	=> \$tst_only,
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
+	'typedefsfile=s'	=> \$typedefsfile,
 	'color!'	=> \$color,
 	'h|help'	=> \$help,
 	'version'	=> \$help
@@ -629,28 +632,43 @@  if ($codespell) {
 
 $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix;
 
+sub read_words {
+	my ($wordsRef, $file) = @_;
+
+	if (open(my $words, '<', $file)) {
+		while (<$words>) {
+			my $line = $_;
+
+			$line =~ s/\s*\n?$//g;
+			$line =~ s/^\s*//g;
+
+			next if ($line =~ m/^\s*#/);
+			next if ($line =~ m/^\s*$/);
+			if ($line =~ /\s/) {
+				print("$file: '$line' invalid - ignored\n");
+				next;
+			}
+
+			$$wordsRef .= '|' if ($$wordsRef ne "");
+			$$wordsRef .= $line;
+		}
+		close($file);
+		return 1;
+	}
+
+	return 0;
+}
+
 my $const_structs = "";
-if (open(my $conststructs, '<', $conststructsfile)) {
-	while (<$conststructs>) {
-		my $line = $_;
+read_words(\$const_structs, $conststructsfile)
+    or warn "No structs that should be const will be found - file '$conststructsfile': $!\n";
 
-		$line =~ s/\s*\n?$//g;
-		$line =~ s/^\s*//g;
-
-		next if ($line =~ m/^\s*#/);
-		next if ($line =~ m/^\s*$/);
-		if ($line =~ /\s/) {
-			print("$conststructsfile: '$line' invalid - ignored\n");
-			next;
-		}
-
-		$const_structs .= '|' if ($const_structs ne "");
-		$const_structs .= $line;
-	}
-	close($conststructsfile);
-} else {
-	warn "No structs that should be const will be found - file '$conststructsfile': $!\n";
+my $typeOtherTypedefs = "";
+if (length($typedefsfile)) {
+	read_words(\$typeOtherTypedefs, $typedefsfile)
+	    or warn "No additional types will be considered - file '$typedefsfile': $!\n";
 }
+$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne "");
 
 sub build_types {
 	my $mods = "(?x:  \n" . join("|\n  ", (@modifierList, @modifierListFile)) . "\n)";