Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   =?UTF-8?q?Niklas=20S=C3=B6derlund?= <niklas.soderlund@corigine.com>
To:     Dwaipayan Ray <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>,
        Joe Perches <joe@perches.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Andy Whitcroft <apw@canonical.com>, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Philippe Schenker <philippe.schenker@toradex.com>,
        Stephen Rothwell <sfr@canb.auug.org.au>
Cc:     oss-drivers@corigine.com,
        =?UTF-8?q?Niklas=20S=C3=B6derlund?= <niklas.soderlund@corigine.com>,
        Simon Horman <simon.horman@corigine.com>,
        Louis Peens <louis.peens@corigine.com>
Subject: [PATCH v7] checkpatch: warn for non-standard fixes tag style
Date:   Wed, 14 Sep 2022 12:02:55 +0200
Message-Id: <20220914100255.1048460-1-niklas.soderlund@corigine.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QUxzRTJIczZzWk8vZkE5M1pjUjBIdmRMVXZQYzJEMC81ZUFja1dTMGdzSE5F?=
 =?utf-8?B?ZllNQk41dW9QZmhLa1JvUFB6ZzRNR1RZUGdnQnVKTWdLVU02c1RVYm4vUmo1?=
 =?utf-8?B?dGsrYkQ1MVFpVUFVaHV3V28rUmJtNHNuV3JibjltbHd2NWtYQ2hJcUpONndZ?=
 =?utf-8?B?RW9hSUFjNzAzM2EvY1l4bTQ3dGtxS01RVVpUNis0aUl1WEVhRFg3YWF1VGFX?=
 =?utf-8?B?d1MyNGZHc21YQzVpWnRISE9FZG5kVVF6d0RsdVF5M242OHB6L0NSV2RnU2tr?=
 =?utf-8?B?QzNUa1dTUUpBc1RuRjlqN2hCc3JtK3FMV2ZkNEpMMWdPZWdHcDVZbFVRbm5D?=
 =?utf-8?B?b1BkQUoyajArTDNqSDQrVmJobjZvQUJtNWN0Nlg0dFpWTU9PQkpReWIra1Jz?=
 =?utf-8?B?RndoUkVhWWJBaXRNdkFsV3Vobmk1SEVFdlNlcXJJdFhmTzZGLzZvQ2w1ekc1?=
 =?utf-8?B?Skw0YmNreVNET1VSTnhYQzhqRitSaFEvdVE2UiswTzRPMnV1aWtYYzVZRFps?=
 =?utf-8?B?bXBsK2JUdVMzU1NjbDhjRytkcFpoVnAxRHhtaVRqRElSYVhVbzhZek5QTWJx?=
 =?utf-8?B?QzhsUmJrbityTEtuZmlRTEc3V0RwT0JKVVBaUWxyRGd4OU1GU1c3MGVIU1hm?=
 =?utf-8?B?OFhaYXgycDV2bWs5VzJVRTRxUm1MS3k3SkxDZ3RQa1pCbTRRaStEZG55b040?=
 =?utf-8?B?RTRQdnh4bWZZbVpGQ1dzZG9pMkFMbFhIUWdmTkVyQzhZUkNhYUgwSWNwRjla?=
 =?utf-8?B?Yk9hQ0lPWHJZTFBxQ1crU1dlV2V6ZE1kSWtuUmQ0eldoZmErb0MvZ25IQVF1?=
 =?utf-8?B?MUlvVUF3ci9pNlpNREEybFFzNlVSNk0zKzQ3ejJiV1BaVTA4QlIzb2R2RGFk?=
 =?utf-8?B?TnJqb21JUkxMWldZaHJZRU5JMnF5Yjdta3dhZS9DTkMwVzA0NUlhYU5TaTBR?=
 =?utf-8?B?Sjcreml1ZmRTaGJPSEI2ZFpFTkxXWm9PaDZZS3dPOFoxWXRGeUFjNDQvMk1z?=
 =?utf-8?B?V3k5bkNHT2lxd2xwVEQrcDErR3BSd0NuZWt0WG81a2t4QjlmYWlCWlU5bjhW?=
 =?utf-8?B?VDM3NW5MUmFjNXF6M0l6ZWc3Q0pRWWtIZWNkUnBnTm1OKys1U09lRmlVT0dm?=
 =?utf-8?B?a1pLNU1BSlJEWG82QVBleWxhTTZJejRCT2NvNjUvY3lZWHM1SWw1N2ZqKzlX?=
 =?utf-8?B?TjYrRk9WSU0rWERYUklRbzBLQzh3Y0Ztc0dQVUk2TFk4b3pBeE5DSVFvS29p?=
 =?utf-8?B?WUI3WXZLNjFFUklHU1FERVNwZnh5N2g2bkZBUGM5RGp0V3c3WUVBVGxVbEJk?=
 =?utf-8?B?bmJXN3dRTUJtL2hJNVByUHYwSnkzMW8xNE9kK2NNdWh1VmhDWjJNcXN5a1Nt?=
 =?utf-8?B?aDN2UUhXWmdpYlBkdk1ueXZ3UnJLeTBNRnd1cDJ5UFFLY1d6V2tvSnBWRGN4?=
 =?utf-8?B?NVViZ1lMaXEyR3ZJUUw3MjlFazRFZjBHMGdINmhvRVg1aWIvZ0YzN0Z2aTE3?=
 =?utf-8?B?bHJRcHBnS29TYW16UlkrN3FVcmkzWURBMUFZSzRGVlp3R0xLT3BDOE1tUXdG?=
 =?utf-8?B?Q1dyWnVxQTY2Yk9RcWw1d2toZ3did2lWVzF3RG01NzlDYnR2Wmw1WEl2Q3px?=
 =?utf-8?B?VFVLdDdnZGIzL2FqUnJSNDhnemorR2NUL2dVRkhqMDFjaXJDa3RXbkFjSW9Y?=
 =?utf-8?B?cVFMTjcyZVhkUGpMcTRzYlBhZEZxOEFERXNuRFBnTlJZMVF0UVg0L1hnWWRt?=
 =?utf-8?B?dS9qWkpaelVGeVRBSkhzdWJUVVg3VjBzUDhvaU9Dd1FnYlptdXc4QXlYZERl?=
 =?utf-8?B?Yk1VOUc1NTFzMmpSeHNQaUVjdzdpV2RtMEwwZzRidE5wRTQ3ZENMSUd6OVZm?=
 =?utf-8?B?RmtxWDdpbjl1VUNPNEhWSU5SWlZpUXphTjdybDBTNlZxcjlDeEFHZE1XM3VW?=
 =?utf-8?B?Zlg3Ym83OUJ5RDhjNjRxSGRza0NnSk1jbzlRaWViR01VQUhxcW44bmprNzQy?=
 =?utf-8?B?NFk1bFh6ZVpsQUd4dFBUdUY2UjRkTVVZZFBHZzlQQjcwMGU4dmw5MktmY3hN?=
 =?utf-8?B?aTBuYnhGRnZLSHZFYkdDWmw0dTB3T2Ezb2VZcXRxTVFqOGFaMjlneUNsSTlD?=
 =?utf-8?B?cHJtQ2lDdFc5blVDakJ3U3BJcmdnNmh1Q3VvRitiVXRQeTUyYVMyaDQyY0pL?=
 =?utf-8?B?QUE9PQ==?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR13MB4690
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Add a warning for fixes tags that does not follow community conventions.

Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Reviewed-by: Louis Peens <louis.peens@corigine.com>
Reviewed-by: Philippe Schenker <philippe.schenker@toradex.com>
Acked-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Acked-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
* Changes since v6
- Update first check to make sure that there is a likely SHA1 of some
  minimum length after the fixes line.
- s/fall in line with community standard/follow community conventions/.
- Improve grammar, thanks Lukas.

* Changes since v5
- Add support for --fix option for checkpatch.pl.

* Changes since v4
- Extend test to cover lines with whitespace before the fixes: tag, e.g.
  match check on /^\s*fixes:?/i.

* Changes since v3
- Add test that title in tag match title of commit referenced by sha1.

* Changes since v2
- Change the pattern to match on 'fixes:?' to catch more malformed
  tags.

* Changes since v1
- Update the documentation wording and add mention one cause of the
  message can be that email program splits the tag over multiple lines.
---
 Documentation/dev-tools/checkpatch.rst |  7 ++++
 scripts/checkpatch.pl                  | 44 ++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index b52452bc2963..c3389c6f3838 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -612,6 +612,13 @@ Commit message
 
     See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
 
+  **BAD_FIXES_TAG**
+    The Fixes: tag is malformed or does not follow the community conventions.
+    This can occur if the tag have been split into multiple lines (e.g., when
+    pasted in an email program with word wrapping enabled).
+
+    See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
+
 
 Comparison style
 ----------------
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 79e759aac543..ddc5c9d730c3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3140,6 +3140,50 @@ 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;
+			my $title_has_quotes = 0;
+
+			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]/);
+
+				# Always strip leading/trailing parens then double quotes if existing
+				$title = substr($title, 1, -1);
+				if ($title =~ /^".*"$/) {
+					$title = substr($title, 1, -1);
+					$title_has_quotes = 1;
+				}
+			}
+
+			my ($cid, $ctitle) = git_commit_info($orig_commit, $id,
+							     $title);
+
+			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) &&
+				    $fix) {
+					$fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")";
+				}
+			}
+		}
+
 # Check email subject for common tools that don't need to be mentioned
 		if ($in_header_lines &&
 		    $line =~ /^Subject:.*\b(?:checkpatch|sparse|smatch)\b[^:]/i) {
-- 
2.37.3