linux-kernel - Re: [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe to show filenames

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150604121804.GP3135@pathway.suse.cz>
Date:	Thu, 4 Jun 2015 14:18:04 +0200
From:	Petr Mladek <pmladek@...e.cz>
To:	Joe Perches <joe@...ches.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Whitcroft <apw@...onical.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] checkpatch: Add --showfile to allow input via pipe
 to show filenames

On Wed 2015-06-03 08:53:40, Joe Perches wrote:
> Using "git diff | ./scripts/checkpatch -" does not have an
> easy mechanism to see the files and lines actually modified.
> 
> Add --showfile to see the file and line specified in the diff.
> 
> When --showfile is used without --terse, the second line of each
> message output is redundant, so it is removed.

> Signed-off-by: Joe Perches <joe@...ches.com>

I like idea but there is a problem, see below.

> ---
>  scripts/checkpatch.pl | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index cef2cd4..1241f99d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -25,6 +25,7 @@ my $chk_patch = 1;
>  my $tst_only;
>  my $emacs = 0;
>  my $terse = 0;
> +my $showfile = 0;
>  my $file = 0;
>  my $check = 0;
>  my $check_orig = 0;
> @@ -66,6 +67,7 @@ Options:
>    --patch                    treat FILE as patchfile (default)
>    --emacs                    emacs compile window format
>    --terse                    one line per report
> +  --showfile                 emit diffed file position, not input file position
>    -f, --file                 treat FILE as regular source file
>    --subjective, --strict     enable more subjective tests
>    --types TYPE(,TYPE2...)    show only these comma separated message types
> @@ -137,6 +139,7 @@ GetOptions(
>  	'patch!'	=> \$chk_patch,
>  	'emacs!'	=> \$emacs,
>  	'terse!'	=> \$terse,
> +	'showfile!'	=> \$showfile,
>  	'f|file!'	=> \$file,
>  	'subjective!'	=> \$check,
>  	'strict!'	=> \$check,
> @@ -1693,6 +1696,12 @@ sub report {
>  	}
>  	$output .= RESET if (-t STDOUT && $color);
>  	$output .= ' ' . $msg . "\n";
> +
> +	if ($showfile) {
> +		my @lines = split("\n", $output, -1);
> +		splice(@lines, 1, 1);
> +		$output = join("\n", @lines);
> +	}
>  	$output = (split('\n', $output))[0] . "\n" if ($terse);
>  
>  	push(our @report, $output);
> @@ -2119,10 +2128,6 @@ sub process {
>  
>  		my $hunk_line = ($realcnt != 0);
>  
> -#make up the handle for any error we report on this line
> -		$prefix = "$filename:$realline: " if ($emacs && $file);
> -		$prefix = "$filename:$linenr: " if ($emacs && !$file);
> -
>  		$here = "#$linenr: " if (!$file);
>  		$here = "#$realline: " if ($file);
>  
> @@ -2152,6 +2157,13 @@ sub process {
>  			$found_file = 1;
>  		}
>  
> +#make up the handle for any error we report on this line
> +		if ($showfile) {
> +			$prefix = "$realfile:$realline: "
> +		} elsif ($emacs) {
> +			$prefix = "$filename:$linenr: ";
> +		}
> +
>  		if ($found_file) {
>  			if ($realfile =~ m@^(drivers/net/|net/)@) {
>  				$check = 1;
> @@ -5606,7 +5618,7 @@ sub process {
>  		ERROR("NOT_UNIFIED_DIFF",
>  		      "Does not appear to be a unified-diff format patch\n");
>  	}
> -	if ($is_patch && $chk_signoff && $signoff == 0) {
> +	if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) {

You might use also "cat $patch | ./scripts/checkpatch -" and in this
case you would want to print the warning.

It still prints invalid filename:linenum when you call:
./scripts/checkpatch 0001-my-commit.patch

A better solution would be to omit the filename:linenum information
for this patch-specific messages or print the patchname instead of
the filename.

Best Regards,
Petr

>  		ERROR("MISSING_SIGN_OFF",
>  		      "Missing Signed-off-by: line(s)\n");
>  	}
> -- 
> 2.1.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/