linux-kernel - Re: [PATCH v2 1/2] checkpatch: warn about novice phrases in commit messages

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: <51bece321fb6cda098e642ebd82ce1d43452e7da.camel@perches.com>
Date: Tue, 22 Jul 2025 20:26:35 -0700
From: Joe Perches <joe@...ches.com>
To: Ignacio Peña <ignacio.pena87@...il.com>, Andy
 Whitcroft <apw@...onical.com>
Cc: Dwaipayan Ray <dwaipayanray1@...il.com>, Lukas Bulwahn
	 <lukas.bulwahn@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] checkpatch: warn about novice phrases in commit
 messages

On Tue, 2025-07-22 at 23:02 -0400, Ignacio Peña wrote:
> Add detection for common phrases that make patches appear less
> confident. A single regex efficiently matches multiple patterns:
> - 'please apply/merge/consider/review'
> - 'hope this helps'
> - 'my first patch/contribution'
> - 'newbie/beginner here'
> - 'not sure if (this is) correct'
> - 'sorry if/for'
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3266,6 +3266,15 @@ sub process {
>  			     "A patch subject line should describe the change not the tool that found it\n" . $herecurr);
>  		}
>  
> +
> +# Check for novice phrases in commit message
> +		if ($in_commit_log && !$non_utf8_charset) {
> +			# Single regex with all phrases, using non-capturing groups as Joe suggested

The comment referencing a suggetion should be removed.

> +			if ($line =~ /\b(?:please\s+(?:apply|merge|consider|review)|hope\s+this\s+helps|my\s+first\s+(?:patch|contribution)|(?:newbie|beginner)\s+here|not\s+sure\s+if\s+(?:this\s+is\s+)?correct|sorry\s+(?:if|for))\b/i) {

I think this is unreadable.
The qr{(...)} was and might be a tad faster as it could be precompiled.