linux-kernel - Re: [PATCH] checkpatch: Minimize checkpatch induced patches...

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: <d3d3b8a9-71de-4adf-d685-0a1cf9562389@de.ibm.com>
Date:   Wed, 14 Sep 2016 19:56:55 +0200
From:   Christian Borntraeger <borntraeger@...ibm.com>
To:     Joe Perches <joe@...ches.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Whitcroft <apw@...onical.com>
Cc:     Greg KH <greg@...ah.com>, Jonathan Corbet <corbet@....net>,
        Josh Triplett <josh@...htriplett.org>,
        "Theodore Ts'o" <tytso@....edu>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] checkpatch: Minimize checkpatch induced patches...

On 09/14/2016 07:51 PM, Joe Perches wrote:
> checkpatch can be a useful tool for patches.
> 
> It can be a much more controversial tool when used on files with the
> -f option for style and whitespace changes for code that is relatively
> stable, obsolete, or for maintained by specific individuals.
> 
> o By default, allow checkpatch to be used with the -f|--file option
>   for files in drivers/staging/
> o Add an undocumented --force command line option to be used together
>   with the -f|--file option to scan any file
> 
> Signed-off-by: Joe Perches <joe@...ches.com>
> cc: Greg KH <greg@...ah.com>
> cc: Jonathan Corbet <corbet@....net>
> cc: Josh Triplett <josh@...htriplett.org>
> cc: Christian Borntraeger <borntraeger@...ibm.com>
> cc: Theodore Ts'o <tytso@....edu>

This will certainly help to reduce the noise. On the other hand I remember Linus
saying something along the line that he does not like the -f parameter (and he
prefers to set this automatically). So while I like the approach I am not happy
enough to ack right now - still looking for a better alternative :-/
> ---
>  scripts/checkpatch.pl | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 0ef3d83..d998a61 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -27,6 +27,7 @@ my $emacs = 0;
>  my $terse = 0;
>  my $showfile = 0;
>  my $file = 0;
> +my $force = 0;
>  my $git = 0;
>  my %git_commits = ();
>  my $check = 0;
> @@ -188,6 +189,7 @@ GetOptions(
>  	'terse!'	=> \$terse,
>  	'showfile!'	=> \$showfile,
>  	'f|file!'	=> \$file,
> +	'force!'	=> \$force,
>  	'g|git!'	=> \$git,
>  	'subjective!'	=> \$check,
>  	'strict!'	=> \$check,
> @@ -893,6 +895,10 @@ if ($git) {
>  my $vname;
>  for my $filename (@ARGV) {
>  	my $FILE;
> +	if (!$force && $file && $filename !~ m@...ivers/staging/@) {
> +		warn "$P: checking '$filename' is not supported\n";
> +		next;
> +	}
>  	if ($git) {
>  		open($FILE, '-|', "git format-patch -M --stdout -1 $filename") ||
>  			die "$P: $filename: git format-patch failed - $!\n";
>