Re: [PATCH] checkpatch: Add check for common mailing list helper checks

From: Joe Perches
Date: Sat Jul 03 2021 - 14:39:53 EST


On Fri, 2021-07-02 at 15:37 -0700, John 'Warthog9' Hawley (VMware)
wrote:
> From: John 'Warthog9' Hawley <warthog9@xxxxxxxxxxxxxx>
>
> Mailing lists in an attempt to try and avoid sending certain
> administrative e-mails to the list, will check the first few lines
> (usually ~10) looking for keywords. If those key words are found it
> shunts the e-mail to the list admin contact instead of potentially
> passing it through to the list.

Perhaps the below is a bit better, but I believe a few of the tests
are going to be tripped a bit too often.

Especially "cancel", "config" and maybe "subscribe" too.

For instance:

$ git log --grep='\bcancel\b' -P -i --pretty=oneline -10000 | wc -l
1693

$ git log --grep='^config\b' -P -i --pretty=oneline -10000 | wc -l
890

$ git log --grep='\bsubscribe\b' -P -i --pretty=oneline -10000 | wc -l
123

---
scripts/checkpatch.pl | 45 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 461d4221e4a4a..fcbcc26da875e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -865,6 +865,37 @@ our $allowed_asm_includes = qr{(?x:
)};
# memory.h: ARM has a custom one

+our $mailing_list_phrases = qr{(?xi:
+ \bcancel\b |
+ \badd\s+me\b |
+ \bdelete\s+me\b |
+ \bremove\s+me\b |
+ \bchange\b.*\baddress\b |
+ \bsubscribe\b |
+ ^sub\b |
+ \bunsubscribe\b |
+ ^unsub\b |
+ ^\s*help\s*$ |
+ ^\s*info\s*$ |
+ ^\s*info\s+\S+\s*$ |
+ ^\s*lists\s*$ |
+ ^\s*which\s*$ |
+ ^\s*which\s+\S+\s*$ |
+ ^\s*index\s*$ |
+ ^\s*index\s+\S+\s*$ |
+ ^\s*who\s*$ |
+ ^\s*who\s+\S+\s*$ |
+ ^\s*get\s+\S+\s*$ |
+ ^\s*get\s+\S+\s+\S+\s*$ |
+ ^\s*approve\b |
+ ^\s*passwd\b |
+ ^\s*newinfo\b |
+ ^\s*config\b |
+ ^\s*newconfig\b |
+ ^\s*writeconfig\b |
+ ^\s*mkdigest\b
+)};
+
# Load common spelling mistakes and build regular expression list.
my $misspellings;
my %spelling_fix;
@@ -2581,6 +2612,7 @@ sub process {
my $has_patch_separator = 0; #Found a --- line
my $has_commit_log = 0; #Encountered lines before patch
my $commit_log_lines = 0; #Number of commit log lines
+ my $commit_log_missing = 0; #Emitted a missing commit message warning
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2852,16 +2884,23 @@ sub process {

$cnt_lines++ if ($realcnt != 0);

+# check if words in the commit message may trip up common mailing list helpers
+# to redirect email to the admin contact
+ if ($in_commit_log && $commit_log_lines < 10 &&
+ $line =~ /($mailing_list_phrases)/) {
+ WARN("MAILING_LIST_HELPER",
+ "Line matches common mailing list helpers and may not be delivered correctly - consider rewording '$1'\n" . $herecurr);
+ }
+
# Verify the existence of a commit log if appropriate
-# 2 is used because a $signature is counted in $commit_log_lines
if ($in_commit_log) {
if ($line !~ /^\s*$/) {
$commit_log_lines++; #could be a $signature
}
- } elsif ($has_commit_log && $commit_log_lines < 2) {
+ } elsif ($has_commit_log && !$commit_log_missing) {
WARN("COMMIT_MESSAGE",
"Missing commit description - Add an appropriate one\n");
- $commit_log_lines = 2; #warn only once
+ $commit_log_missing = 1; #warn only once
}

# Check if the commit log has what seems like a diff which can confuse patch