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-next>] [day] [month] [year] [list]
Message-Id: <20241025-checkpatch-fixes-commit-v2-1-4bc4f06d37b3@gmail.com>
Date: Fri, 25 Oct 2024 19:43:19 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Andy Whitcroft <apw@...onical.com>, Joe Perches <joe@...ches.com>, 
 Dwaipayan Ray <dwaipayanray1@...il.com>, 
 Lukas Bulwahn <lukas.bulwahn@...il.com>, Simon Horman <horms@...nel.org>, 
 Niklas Söderlund <niklas.soderlund+renesas@...natech.se>, 
 Andrew Morton <akpm@...ux-foundation.org>, 
 Philippe Schenker <philippe.schenker@...adex.com>
Cc: Louis Peens <louis.peens@...igine.com>, linux-kernel@...r.kernel.org, 
 Tamir Duberstein <tamird@...il.com>
Subject: [PATCH v2] checkpatch: always parse orig_commit in fixes tag

Do not require the presence of `$balanced_parens` to get the commit SHA;
this allows a `Fixes: deadbeef` tag to get a correct suggestion rather
than a suggestion containing a reference to HEAD.

Given this patch:
```
>From c10a7d25e68ffd6222db2b63d86efa2fba04008b Mon Sep 17 00:00:00 2001
From: Tamir Duberstein <tamird@...il.com>
Date: Fri, 25 Oct 2024 19:30:51 -0400
Subject: [PATCH] Test patch

This is a test patch.

Fixes: bd17e036b495
Signed-off-by: Tamir Duberstein <tamird@...il.com>
---
Changes in v2:
- Added sample input and before and after output to patch description.
- Link to v1: https://lore.kernel.org/r/20241019-checkpatch-fixes-commit-v1-1-0d0cde18ce69@gmail.com
---
 new-file | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 new-file

diff --git a/new-file b/new-file
new file mode 100644
index 000000000000..90b67dda6d59
--- /dev/null
+++ b/new-file
@@ -0,0 +1 @@
+Test.
--
2.47.0
```

Before:
```
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
Fixes: bd17e036b495

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
new file mode 100644

total: 0 errors, 2 warnings, 1 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] Test patch" has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
```

After:
```
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
Fixes: bd17e036b495

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
new file mode 100644

total: 0 errors, 2 warnings, 1 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] Test patch" has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
```

The diff between the outputs:
```
diff --git a/before b/after
index d0cfdb243a1f..6f97b4c53080 100644
--- a/before
+++ b/after
@@ -1,4 +1,4 @@
-WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: c10a7d25e68f ("Test patch")'
+WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")'
 #8:
 Fixes: bd17e036b495
```

The prior behavior incorrectly suggested the patch's own SHA and title
line rather than the referenced commit's. This fixes that.

Ironically this:

Fixes: bd17e036b495 ("checkpatch: warn for non-standard fixes tag style")
Signed-off-by: Tamir Duberstein <tamird@...il.com>
---
 scripts/checkpatch.pl | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4427572b24771ddb1f2bf3de3176f9437f643951..b03d526e4c454af7a42624155175e109d499762f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3209,36 +3209,31 @@ sub process {
 
 # Check Fixes: styles is correct
 		if (!$in_header_lines &&
-		    $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) {
-			my $orig_commit = "";
-			my $id = "0123456789ab";
-			my $title = "commit title";
-			my $tag_case = 1;
-			my $tag_space = 1;
-			my $id_length = 1;
-			my $id_case = 1;
+		    $line =~ /^\s*(fixes:?)\s*(?:commit\s*)?([0-9a-f]{5,40})(?:\s*($balanced_parens))?/i) {
+			my $tag = $1;
+			my $orig_commit = $2;
+			my $title;
 			my $title_has_quotes = 0;
 			$fixes_tag = 1;
-
-			if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) {
-				my $tag = $1;
-				$orig_commit = $2;
-				$title = $3;
-
-				$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_case = 0 if ($orig_commit !~ /[A-F]/);
-
+			if (defined $3) {
 				# Always strip leading/trailing parens then double quotes if existing
-				$title = substr($title, 1, -1);
+				$title = substr($3, 1, -1);
 				if ($title =~ /^".*"$/) {
 					$title = substr($title, 1, -1);
 					$title_has_quotes = 1;
 				}
+			} else {
+				$title = "commit title"
 			}
 
+
+			my $tag_case = not ($tag eq "Fixes:");
+			my $tag_space = not ($line =~ /^fixes:? [0-9a-f]{5,40} ($balanced_parens)/i);
+
+			my $id_length = not ($orig_commit =~ /^[0-9a-f]{12}$/i);
+			my $id_case = not ($orig_commit !~ /[A-F]/);
+
+			my $id = "0123456789ab";
 			my ($cid, $ctitle) = git_commit_info($orig_commit, $id,
 							     $title);
 

---
base-commit: c71f8fb4dc911022748a378b16aad1cc9b43aad8
change-id: 20241019-checkpatch-fixes-commit-aa014fe6d0b3

Best regards,
-- 
Tamir Duberstein <tamird@...il.com>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ