linux-kernel - [RFC] checkpatch.pl Is "does MAINTAINERS need updating" check broken?

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20201119155217.GA631033@johnsa1-desk.jf.intel.com>
Date:   Thu, 19 Nov 2020 07:52:17 -0800
From:   "Andersen, John" <john.s.andersen@...el.com>
To:     linux-kernel@...r.kernel.org
Cc:     Andy Whitcroft <apw@...onical.com>, Joe Perches <joe@...ches.com>
Subject: [RFC] checkpatch.pl Is "does MAINTAINERS need updating" check broken?

Hi,

I just noticed that patches which have "added, moved or deleted file(s)" and
updated MAINTAINERS still trigger the following warning:

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

Here's two examples using existing patches

$ git show --format=email a7305e684fcfb33029fe3d0af6b7d8dc4c8ca7a1 | ./scripts/checkpatch.pl
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#103:
rename from drivers/power/avs/qcom-cpr.c

total: 0 errors, 1 warnings, 39 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] PM: AVS: qcom-cpr: Move the driver to the qcom specific" has style problems, please review.

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

$ git show --format=email bca815d620544c27288abf4841e39922d694425c | ./scripts/checkpatch.pl
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#89:
rename from drivers/power/avs/smartreflex.c

total: 0 errors, 1 warnings, 44 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] PM: AVS: smartreflex Move driver to soc specific drivers" 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 applying the patch below it seems that checkpatch now picks up the change
to MAINTAINERS correctly.

$ git show --format=email a7305e684fcfb33029fe3d0af6b7d8dc4c8ca7a1 | ./scripts/checkpatch.pl
total: 0 errors, 0 warnings, 39 lines checked

"[PATCH] PM: AVS: qcom-cpr: Move the driver to the qcom specific" has no obvious style problems and is ready for submission.

$ git show --format=email bca815d620544c27288abf4841e39922d694425c | ./scripts/checkpatch.pl
total: 0 errors, 0 warnings, 44 lines checked

"[PATCH] PM: AVS: smartreflex Move driver to soc specific drivers" has no obvious style problems and is ready for submission.

I am not sure if this is a "doesn't work on my machine" situation or if there is
really an issue here. Please let me know your thoughts.

I didn't do much digging into this but found that simply changing the regex
wasn't enough. I had to put a separate loop above the main loop. I've never
touched pearl so that's they this is such a dumb fix.

Thanks,
John

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..74e27cd64c58 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2510,6 +2510,14 @@ sub process {

 	$prefix = '';

+# Check if MAINTAINERS is being updated.  If so, there's probably no need to
+# emit the "does MAINTAINERS need updating?" message on file add/move/delete
+	foreach my $line (@lines) {
+		if ($line =~ /\+\+\+ b\/MAINTAINERS$/) {
+			$reported_maintainer_file = 1;
+		}
+	}
+
 	$realcnt = 0;
 	$linenr = 0;
 	$fixlinenr = -1;
@@ -2744,12 +2752,6 @@ sub process {
 			$in_commit_log = 0;
 		}

-# Check if MAINTAINERS is being updated.  If so, there's probably no need to
-# emit the "does MAINTAINERS need updating?" message on file add/move/delete
-		if ($line =~ /^\s*MAINTAINERS\s*\|/) {
-			$reported_maintainer_file = 1;
-		}
-
 # Check signature styles
 		if (!$in_header_lines &&
 		    $line =~ /^(\s*)([a-z0-9_-]+by:|$signature_tags)(\s*)(.*)/i) {