linux-kernel - Re: [PATCH] checkpatch.pl: Relax commit ID check to allow more than 12 chars

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] [day] [month] [year] [list]
Message-ID: <Y+JkbGmuU9hmuc80@probook>
Date:   Tue, 7 Feb 2023 15:47:08 +0100
From:   Jonathan Neuschäfer <j.neuschaefer@....net>
To:     Joe Perches <joe@...ches.com>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Jonathan Neuschäfer <j.neuschaefer@....net>,
        linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH] checkpatch.pl: Relax commit ID check to allow more than
 12 chars

On Mon, Feb 06, 2023 at 04:25:19AM -0800, Joe Perches wrote:
> On Mon, 2023-02-06 at 12:54 +0100, Geert Uytterhoeven wrote:
> > Hi Joe,
> 
> rehi Geert
> 
> maybe:
> ---

At a quick glance, this looks reasonable to me. Feel free to take over
the patch and send a real v2.


Thanks,
Jonathan


>  scripts/checkpatch.pl | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index bd44d12965c98..55267ee6b1190 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -36,6 +36,8 @@ my $showfile = 0;
>  my $file = 0;
>  my $git = 0;
>  my %git_commits = ();
> +my $git_sha1_min = 12;
> +my $git_sha1_max = 14;
>  my $check = 0;
>  my $check_orig = 0;
>  my $summary = 1;
> @@ -1230,7 +1232,13 @@ sub git_commit_info {
>  		 $lines[0] =~ /^fatal: bad object $commit/) {
>  		$id = undef;
>  	} else {
> -		$id = substr($lines[0], 0, 12);
> +		my $len = length($commit);
> +		if ($len < $git_sha1_min) {
> +			$len = $git_sha1_min;
> +		} elsif ($len > $git_sha1_max) {
> +			$len = $git_sha1_max;
> +		}
> +		$id = substr($lines[0], 0, $len);
>  		$desc = substr($lines[0], 41);
>  	}
>  
> @@ -1297,7 +1305,7 @@ for my $filename (@ARGV) {
>  	if ($filename eq '-') {
>  		$vname = 'Your patch';
>  	} elsif ($git) {
> -		$vname = "Commit " . substr($filename, 0, 12) . ' ("' . $git_commits{$filename} . '")';
> +		$vname = "Commit " . substr($filename, 0, $git_sha1_min) . ' ("' . $git_commits{$filename} . '")';
>  	} else {
>  		$vname = $filename;
>  	}
> @@ -3191,7 +3199,7 @@ sub process {
>  				$tag_case = 0 if $tag eq "Fixes:";
>  				$tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i);
>  
> -				$id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i);
> +				$id_length = 0 if ($orig_commit =~ /^[0-9a-f]{$git_sha1_min,$git_sha1_max}$/i);
>  				$id_case = 0 if ($orig_commit !~ /[A-F]/);
>  
>  				# Always strip leading/trailing parens then double quotes if existing
> @@ -3208,7 +3216,7 @@ sub process {
>  			if ($ctitle ne $title || $tag_case || $tag_space ||
>  			    $id_length || $id_case || !$title_has_quotes) {
>  				if (WARN("BAD_FIXES_TAG",
> -				     "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) &&
> +				     "Please use correct Fixes: style 'Fixes: <$git_sha1_min to $git_sha1_max chars of sha1> (\"<title line>\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) &&
>  				    $fix) {
>  					$fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")";
>  				}
> @@ -3300,9 +3308,9 @@ sub process {
>  		    $line !~ /^This reverts commit [0-9a-f]{7,40}/ &&
>  		    (($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i ||
>  		      ($line =~ /\bcommit\s*$/i && defined($rawlines[$linenr]) && $rawlines[$linenr] =~ /^\s*[0-9a-f]{5,}\b/i)) ||
> -		     ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i &&
> -		      $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i &&
> -		      $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) {
> +		     ($line =~ /(?:\s|^)[0-9a-f]{$git_sha1_min,40}(?:[\s"'\(\[]|$)/i &&
> +		      $line !~ /[\<\[][0-9a-f]{$git_sha1_min,40}[\>\]]/i &&
> +		      $line !~ /\bfixes:\s*[0-9a-f]{$git_sha1_min,40}/i))) {
>  			my $init_char = "c";
>  			my $orig_commit = "";
>  			my $short = 1;
> @@ -3340,11 +3348,11 @@ sub process {
>  			if ($input =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
>  				$init_char = $1;
>  				$orig_commit = lc($2);
> -				$short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i);
> +				$short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{$git_sha1_min,40}/i);
>  				$long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i);
>  				$space = 0 if ($input =~ /\bcommit [0-9a-f]/i);
>  				$case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
> -			} elsif ($input =~ /\b([0-9a-f]{12,40})\b/i) {
> +			} elsif ($input =~ /\b([0-9a-f]{$git_sha1_min,40})\b/i) {
>  				$orig_commit = lc($1);
>  			}
>  
> @@ -3355,7 +3363,7 @@ sub process {
>  			    ($short || $long || $space || $case || ($orig_desc ne $description) || !$has_quotes) &&
>  			    $last_git_commit_id_linenr != $linenr - 1) {
>  				ERROR("GIT_COMMIT_ID",
> -				      "Please use git commit description style 'commit <12+ chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herectx);
> +				      "Please use git commit description style 'commit <$git_sha1_min to $git_sha1_max chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herectx);
>  			}
>  			#don't report the next line if this line ends in commit and the sha1 hash is the next line
>  			$last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)