[PATCH] checkpatch: Validate "Fixes:" references

Bjorn Andersson posted 1 patch 4 years, 3 months ago
scripts/checkpatch.pl | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
[PATCH] checkpatch: Validate "Fixes:" references
Posted by Bjorn Andersson 4 years, 3 months ago
checkpatch guards about non-compliant or invalid commit references
embedded in the commit message, but a similar set of mistakes can be
made when referencing commits in "Fixes:".

Add sanity checks for references to missing commits, short hashes,
improper spacing, improper case or issues with the commit description.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 scripts/checkpatch.pl | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 577e02998701..b602ab50fc5c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3273,6 +3273,42 @@ sub process {
 			$last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
 		}
 
+# Check for valid Fixes:
+		if ($line =~ /^fixes:\s*([0-9a-f]{5,})/i) {
+			my $orig_commit = lc($1);
+
+			my $case = 1;
+			my $short = 1;
+			my $space = 1;
+			my $has_quotes = 0;
+			my $id = '0123456789ab';
+			my $orig_desc = "commit description";
+			my $description = "";
+
+			$case = 0 if ($line =~ /^Fixes:\s+[0-9a-f]{5,40}[^A-F]/);
+			$short = 0 if ($line =~ /\bfixes:\s*[0-9a-f]{12,40}/i);
+			$space = 0 if ($line =~ /\bfixes: [0-9a-f]{5,} \(/i);
+
+			if ($line =~ /\bfixes:\s+[0-9a-f]{5,}\s*($balanced_parens)/i) {
+				$orig_desc = substr($1, 1, -1);
+				if ($orig_desc =~ /^".*"$/) {
+					$orig_desc = substr($orig_desc, 1, -1);
+					$has_quotes = 1;
+				}
+			}
+
+			($id, $description) = git_commit_info($orig_commit,
+							      $id, $orig_desc);
+
+			if (defined($id) && ($short || $space || $case || ($orig_desc ne $description) || !$has_quotes)) {
+				ERROR("GIT_COMMIT_ID",
+				      "Please use git commit description style 'Fixes: <12+ chars of sha1> (\"<title line>\")' - ie: 'Fixes: $id (\"$description\")'\n" . $herecurr);
+			} elsif (!defined($id)) {
+				WARN("UNKNOWN_COMMIT_ID",
+				     "Unknown commit id '$orig_commit', maybe rebased or not pulled?\n" . $herecurr);
+			}
+		}
+
 # Check for added, moved or deleted files
 		if (!$reported_maintainer_file && !$in_commit_log &&
 		    ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||
-- 
2.33.1
Re: [PATCH] checkpatch: Validate "Fixes:" references
Posted by Joe Perches 4 years, 3 months ago
On Mon, 2022-03-21 at 11:07 -0700, Bjorn Andersson wrote:
> checkpatch guards about non-compliant or invalid commit references
> embedded in the commit message, but a similar set of mistakes can be
> made when referencing commits in "Fixes:".
> 
> Add sanity checks for references to missing commits, short hashes,
> improper spacing, improper case or issues with the commit description.

nak.

The idea is ok but:

This basically duplicates the commit test block so should this
should be either a function or integrated into the commit test.

> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  scripts/checkpatch.pl | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 577e02998701..b602ab50fc5c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3273,6 +3273,42 @@ sub process {
>  			$last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
>  		}
>  
> +# Check for valid Fixes:
> +		if ($line =~ /^fixes:\s*([0-9a-f]{5,})/i) {
> +			my $orig_commit = lc($1);
> +
> +			my $case = 1;
> +			my $short = 1;
> +			my $space = 1;
> +			my $has_quotes = 0;
> +			my $id = '0123456789ab';
> +			my $orig_desc = "commit description";
> +			my $description = "";
> +
> +			$case = 0 if ($line =~ /^Fixes:\s+[0-9a-f]{5,40}[^A-F]/);
> +			$short = 0 if ($line =~ /\bfixes:\s*[0-9a-f]{12,40}/i);
> +			$space = 0 if ($line =~ /\bfixes: [0-9a-f]{5,} \(/i);
> +
> +			if ($line =~ /\bfixes:\s+[0-9a-f]{5,}\s*($balanced_parens)/i) {
> +				$orig_desc = substr($1, 1, -1);
> +				if ($orig_desc =~ /^".*"$/) {
> +					$orig_desc = substr($orig_desc, 1, -1);
> +					$has_quotes = 1;
> +				}
> +			}
> +
> +			($id, $description) = git_commit_info($orig_commit,
> +							      $id, $orig_desc);
> +
> +			if (defined($id) && ($short || $space || $case || ($orig_desc ne $description) || !$has_quotes)) {
> +				ERROR("GIT_COMMIT_ID",
> +				      "Please use git commit description style 'Fixes: <12+ chars of sha1> (\"<title line>\")' - ie: 'Fixes: $id (\"$description\")'\n" . $herecurr);
> +			} elsif (!defined($id)) {
> +				WARN("UNKNOWN_COMMIT_ID",
> +				     "Unknown commit id '$orig_commit', maybe rebased or not pulled?\n" . $herecurr);
> +			}
> +		}
> +
>  # Check for added, moved or deleted files
>  		if (!$reported_maintainer_file && !$in_commit_log &&
>  		    ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||