| 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
| ||
|
Message-Id: <20231023102846.14830-1-przemyslaw.kitszel@intel.com>
Date: Mon, 23 Oct 2023 12:28:46 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Andy Whitcroft <apw@...onical.com>, Joe Perches <joe@...ches.com>,
Dwaipayan Ray <dwaipayanray1@...il.com>,
Lukas Bulwahn <lukas.bulwahn@...il.com>,
Sean Christopherson <seanjc@...gle.com>,
workflows@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Jonathan Corbet <corbet@....net>,
linux-doc@...r.kernel.org,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Jacob Keller <jacob.e.keller@...el.com>
Subject: [PATCH] checkpatch: allow tags between co-developed-by and their sign-off
Allow additional tags between Co-developed-by: and Signed-off-by:.
Removing the "immediately" word from the doc is a great summary of the
change - there is no need for the two tags to be glued together, barring
ease of checkpatch implementation.
Additional tags between Co-developed-by and corresponding Signed-off-by
could include Reviewed-by tags collected by Submitter, which is also
a Co-developer, but should sign-off at the very end of tags provided by
the Submitter.
Two sets of perl %hashes introduced to keep both (int) line numbers and
(string) messages handy for warning reporting, while keeping it correct
across 100+ line long commit messages.
Mateusz Polchlopek <mateusz.polchlopek@...el.com> has reported this to me.
Bump severity of missing SoB to ERROR, while that piece of code needs
touch anyway.
changelog:
v2: update also the doc, slight reword of commitmsg,
added workflows & doc MLs;
Links:
v1: https://lore.kernel.org/all/20231020132156.37882-1-przemyslaw.kitszel@intel.com
Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
---
Documentation/process/5.Posting.rst | 2 +-
Documentation/process/submitting-patches.rst | 6 ++--
scripts/checkpatch.pl | 36 +++++++++++++-------
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
index de4edd42d5c0..5dbc874de0f4 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -243,7 +243,7 @@ The tags in common use are:
- Co-developed-by: states that the patch was co-created by several developers;
it is a used to give attribution to co-authors (in addition to the author
attributed by the From: tag) when multiple people work on a single patch.
- Every Co-developed-by: must be immediately followed by a Signed-off-by: of
+ Every Co-developed-by: must be followed by a Signed-off-by: of
the associated co-author. Details and examples can be found in
:ref:`Documentation/process/submitting-patches.rst <submittingpatches>`.
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index efac910e2659..f07521fdb287 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -489,7 +489,7 @@ have been included in the discussion.
Co-developed-by: states that the patch was co-created by multiple developers;
it is used to give attribution to co-authors (in addition to the author
attributed by the From: tag) when several people work on a single patch. Since
-Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
+Co-developed-by: denotes authorship, every Co-developed-by: must be
followed by a Signed-off-by: of the associated co-author. Standard sign-off
procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
chronological history of the patch insofar as possible, regardless of whether
@@ -509,16 +509,18 @@ Example of a patch submitted by the From: author::
Signed-off-by: Second Co-Author <second@...uthor.example.org>
Signed-off-by: From Author <from@...hor.example.org>
-Example of a patch submitted by a Co-developed-by: author::
+Example of a patch submitted by a Co-developed-by: author, who also collected
+a Reviewed-by: tag posted for earlier version::
From: From Author <from@...hor.example.org>
<changelog>
Co-developed-by: Random Co-Author <random@...uthor.example.org>
Signed-off-by: Random Co-Author <random@...uthor.example.org>
Signed-off-by: From Author <from@...hor.example.org>
Co-developed-by: Submitting Co-Author <sub@...uthor.example.org>
+ Reviewed-by: Some Reviewer <srev@...ther.example.org>
Signed-off-by: Submitting Co-Author <sub@...uthor.example.org>
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7d16f863edf1..0400bf092bfa 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2682,6 +2682,10 @@ sub process {
my $suppress_statement = 0;
my %signatures = ();
+ my %signoffs = ();
+ my %signoffs_msg = ();
+ my %codevs = ();
+ my %codevs_msg = ();
# Pre-scan the patch sanitizing the lines.
# Pre-scan the patch looking for any __setup documentation.
@@ -2967,11 +2971,13 @@ sub process {
if ($line =~ /^\s*signed-off-by:\s*(.*)/i) {
$signoff++;
$in_commit_log = 0;
+ my $ctx = $1;
+ $signoffs{$ctx} = $linenr;
+ $signoffs_msg{$ctx} = $herecurr;
if ($author ne '' && $authorsignoff != 1) {
- if (same_email_addresses($1, $author)) {
+ if (same_email_addresses($ctx, $author)) {
$authorsignoff = 1;
} else {
- my $ctx = $1;
my ($email_name, $email_comment, $email_address, $comment1) = parse_email($ctx);
my ($author_name, $author_comment, $author_address, $comment2) = parse_email($author);
@@ -3158,22 +3164,15 @@ sub process {
$signatures{$sig_nospace} = 1;
}
-# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email
+# Collect Co-developed-by: to check if each is backed up by Signed-off-by: with
+# the same name and email. Checks are made after main loop.
if ($sign_off =~ /^co-developed-by:$/i) {
if ($email eq $author) {
WARN("BAD_SIGN_OFF",
"Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . $herecurr);
}
- if (!defined $lines[$linenr]) {
- WARN("BAD_SIGN_OFF",
- "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr);
- } elsif ($rawlines[$linenr] !~ /^signed-off-by:\s*(.*)/i) {
- WARN("BAD_SIGN_OFF",
- "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr . $rawlines[$linenr] . "\n");
- } elsif ($1 ne $email) {
- WARN("BAD_SIGN_OFF",
- "Co-developed-by and Signed-off-by: name/email do not match\n" . $herecurr . $rawlines[$linenr] . "\n");
- }
+ $codevs{$email} = $linenr;
+ $codevs_msg{$email} = $herecurr;
}
# check if Reported-by: is followed by a Closes: tag
@@ -7712,6 +7711,17 @@ sub process {
"From:/Signed-off-by: email subaddress mismatch: $sob_msg\n");
}
}
+ # check if each Co-developed-by tag is backed up by Sign-off,
+ # warn if Co-developed-by tag was put after a Signed-off-by tag
+ foreach my $codev (keys %codevs) {
+ if (!$signoffs{$codev}) {
+ ERROR("BAD_SIGN_OFF",
+ "Co-developed-by: must be followed by Signed-off-by:\n" . $codevs_msg{$codev});
+ } elsif ($signoffs{$codev} <= $codevs{$codev}) {
+ WARN("BAD_SIGN_OFF",
+ "Co-developed-by: must be followed by Signed-off-by:, but was placed after it\n" . $signoffs_msg{$codev} . $codevs_msg{$codev});
+ }
+ }
}
print report_dump();
--
2.38.1
Powered by blists - more mailing lists