public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking
@ 2014-04-08 13:25 Rob Herring
  2014-04-08 13:25 ` [PATCH 2/2] checkpatch: skip directories in file checking mode Rob Herring
  2014-04-08 15:03 ` [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking Joe Perches
  0 siblings, 2 replies; 4+ messages in thread
From: Rob Herring @ 2014-04-08 13:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rob Herring, Florian Vaussard, Joe Perches, Andy Whitcroft,
	Andrew Morton

From: Rob Herring <robh@kernel.org>

We attempt to search for compatible strings which use a variable
token in the documented name such as <chip> or <soc>. While this
was attempted to be handled, it's utterly broken.

The desired forms of matching are:

vendor,<chip>-*
vendor,name<part#>-*

For <chip>, lower case characters and numbers are permitted. For
<part#>, only numeric values are allowed.

With this change, the number of missing compatible strings reported in
arch/arm/boot/dts is reduced from 1071 to 960.

Reported-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Florian Vaussard <florian.vaussard@epfl.ch>
Cc: Joe Perches <joe@perches.com>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 scripts/checkpatch.pl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 34eb216..0e960b1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2093,8 +2093,11 @@ sub process {
 
 			foreach my $compat (@compats) {
 				my $compat2 = $compat;
-				$compat2 =~ s/\,[a-z]*\-/\,<\.\*>\-/;
-				`grep -Erq "$compat|$compat2" $dt_path`;
+				$compat2 =~ s/\,[a-zA-Z0-9]*\-/\,<\.\*>\-/;
+				my $compat3 = $compat;
+				$compat3 =~ s/\,([a-z]*)[0-9]*\-/\,$1<\.\*>\-/;
+				print "$compat3\n";
+				`grep -Erq "$compat|$compat2|$compat3" $dt_path`;
 				if ( $? >> 8 ) {
 					WARN("UNDOCUMENTED_DT_STRING",
 					     "DT compatible string \"$compat\" appears un-documented -- check $dt_path\n" . $herecurr);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] checkpatch: skip directories in file checking mode
  2014-04-08 13:25 [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking Rob Herring
@ 2014-04-08 13:25 ` Rob Herring
  2014-04-08 15:06   ` Joe Perches
  2014-04-08 15:03 ` [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking Joe Perches
  1 sibling, 1 reply; 4+ messages in thread
From: Rob Herring @ 2014-04-08 13:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rob Herring, Andy Whitcroft, Joe Perches, Andrew Morton

From: Rob Herring <robh@kernel.org>

Running checkpatch.pl on a list of files using a wildcard will exit if
a directory is encountered. For example:

$ scripts/checkpatch.pl -f arch/*
diff: arch/alpha/null: No such file or directory

The correct operation is arch/Kconfig should be checked. Fix this by
skipping files which are not regular files.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 scripts/checkpatch.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0e960b1..b646b95 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -552,6 +552,7 @@ my $vname;
 for my $filename (@ARGV) {
 	my $FILE;
 	if ($file) {
+		next if (!(-f $filename));
 		open($FILE, '-|', "diff -u /dev/null $filename") ||
 			die "$P: $filename: diff failed - $!\n";
 	} elsif ($filename eq '-') {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking
  2014-04-08 13:25 [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking Rob Herring
  2014-04-08 13:25 ` [PATCH 2/2] checkpatch: skip directories in file checking mode Rob Herring
@ 2014-04-08 15:03 ` Joe Perches
  1 sibling, 0 replies; 4+ messages in thread
From: Joe Perches @ 2014-04-08 15:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, Rob Herring, Florian Vaussard, Andy Whitcroft,
	Andrew Morton

On Tue, 2014-04-08 at 08:25 -0500, Rob Herring wrote:
> We attempt to search for compatible strings which use a variable
> token in the documented name such as <chip> or <soc>. While this
> was attempted to be handled, it's utterly broken.

Looks like you've left some debugging stuff in this patch.

> The desired forms of matching are:
> 
> vendor,<chip>-*
> vendor,name<part#>-*

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 34eb216..0e960b1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2093,8 +2093,11 @@ sub process {
>  
>  			foreach my $compat (@compats) {
>  				my $compat2 = $compat;
> -				$compat2 =~ s/\,[a-z]*\-/\,<\.\*>\-/;
> -				`grep -Erq "$compat|$compat2" $dt_path`;
> +				$compat2 =~ s/\,[a-zA-Z0-9]*\-/\,<\.\*>\-/;
> +				my $compat3 = $compat;
> +				$compat3 =~ s/\,([a-z]*)[0-9]*\-/\,$1<\.\*>\-/;
> +				print "$compat3\n";

debugging code?

> +				`grep -Erq "$compat|$compat2|$compat3" $dt_path`;
>  				if ( $? >> 8 ) {
>  					WARN("UNDOCUMENTED_DT_STRING",
>  					     "DT compatible string \"$compat\" appears un-documented -- check $dt_path\n" . $herecurr);




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] checkpatch: skip directories in file checking mode
  2014-04-08 13:25 ` [PATCH 2/2] checkpatch: skip directories in file checking mode Rob Herring
@ 2014-04-08 15:06   ` Joe Perches
  0 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2014-04-08 15:06 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel, Rob Herring, Andy Whitcroft, Andrew Morton

On Tue, 2014-04-08 at 08:25 -0500, Rob Herring wrote:
> From: Rob Herring <robh@kernel.org>
> 
> Running checkpatch.pl on a list of files using a wildcard will exit if
> a directory is encountered. For example:
> 
> $ scripts/checkpatch.pl -f arch/*
> diff: arch/alpha/null: No such file or directory
> 
> The correct operation is arch/Kconfig should be checked. Fix this by
> skipping files which are not regular files.

<shrug>

You could make the same argument that it should
skip .o files if you fed it to checkpatch on the
command line.

I think the correct thing is to feed only files
you want scanned to checkpatch.

git ls-files arch/ | xargs ./scripts/checkpatch.pl -f




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-04-08 15:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-08 13:25 [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking Rob Herring
2014-04-08 13:25 ` [PATCH 2/2] checkpatch: skip directories in file checking mode Rob Herring
2014-04-08 15:06   ` Joe Perches
2014-04-08 15:03 ` [PATCH 1/2] checkpatch: fix wildcard DT compatible string checking Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox