[PATCH] scripts/checkpatch: warn on duplicate trailers and self-review

marcandre.lureau@redhat.com posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260727161157.2112984-1-marcandre.lureau@redhat.com
Maintainers: Chao Liu <chao.liu@processmission.com>
scripts/checkpatch.pl | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
[PATCH] scripts/checkpatch: warn on duplicate trailers and self-review
Posted by marcandre.lureau@redhat.com 2 months ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Detect two common trailer mistakes:

- Duplicate trailers: any trailer (Signed-off-by, Fixes, Resolves,
  Cc, etc.) appearing more than once with the same value in a commit
  message.

- Self-review: a Reviewed-by, Tested-by, or Acked-by tag from the
  same person as the patch author, which provides no independent
  review value.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/checkpatch.pl | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 03f35e75012c..a9326297d626 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1539,6 +1539,8 @@ sub process {
 
 	our $clean = 1;
 	my $signoff = 0;
+	my $author_email = '';
+	my %commit_trailers = ();
 	my $is_patch = 0;
 
 	my $in_header_lines = $file ? 0 : 1;
@@ -1795,6 +1797,14 @@ sub process {
 		    ERROR("Author email address is mangled by the mailing list\n" . $herecurr);
 		}
 
+# Extract author email for trailer checks
+		if ($in_header_lines && $line =~ /^(?:Author|From):\s*(.*)/) {
+			my $author_info = $1;
+			if ($author_info =~ /<([^>]+)>/) {
+				$author_email = $1;
+			}
+		}
+
 #check the patch for a signoff:
 		if ($line =~ /^\s*signed-off-by:/i) {
 			# This is a signoff, if ugly, so do not double report.
@@ -1811,6 +1821,28 @@ sub process {
 			}
 		}
 
+# Check for duplicate trailers and self-review
+		if (!$in_header_lines &&
+		    $line =~ /^\s*([A-Z][a-zA-Z]*(?:-[a-zA-Z]+)*):\s*(.+)/) {
+			my $trailer_type = $1;
+			my $trailer_value = $2;
+			$trailer_value =~ s/\s+$//;
+			my $trailer_key = lc("$trailer_type: $trailer_value");
+
+			if (exists $commit_trailers{$trailer_key}) {
+				WARN("Duplicate '$trailer_type' trailer\n" .
+					$herecurr);
+			}
+			$commit_trailers{$trailer_key} = 1;
+
+			if ($trailer_type =~ /^(?:Reviewed|Tested|Acked)-by$/ &&
+			    $author_email ne '' &&
+			    $trailer_value =~ /<\Q$author_email\E>/i) {
+				WARN("$trailer_type from the patch author\n" .
+					$herecurr);
+			}
+		}
+
 # Check SPDX-License-Identifier references a permitted license
 		if (($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) &&
 			$rawline !~ /^-/) {
-- 
2.55.0


Re: [PATCH] scripts/checkpatch: warn on duplicate trailers and self-review
Posted by Paolo Bonzini 3 weeks, 2 days ago
Queued, thanks.

Technically, a duplicate Signed-off-by makes some sense, but it's still worth
warning about it.

Paolo
Re: [PATCH] scripts/checkpatch: warn on duplicate trailers and self-review
Posted by Daniel P. Berrangé 3 weeks, 2 days ago
On Mon, Jul 27, 2026 at 08:11:56PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Detect two common trailer mistakes:
> 
> - Duplicate trailers: any trailer (Signed-off-by, Fixes, Resolves,
>   Cc, etc.) appearing more than once with the same value in a commit
>   message.
> 
> - Self-review: a Reviewed-by, Tested-by, or Acked-by tag from the
>   same person as the patch author, which provides no independent
>   review value.

That could happen if someone starts a patch, then someone else
takes over and does more, then the original author tests/reviews
the result. Since this is merely a warning though, I think that
its fine to ignore.

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/checkpatch.pl | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|


Re: [PATCH] scripts/checkpatch: warn on duplicate trailers and self-review
Posted by Marc-André Lureau 1 month, 1 week ago
Hi

On Mon, Jul 27, 2026 at 8:12 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Detect two common trailer mistakes:
>
> - Duplicate trailers: any trailer (Signed-off-by, Fixes, Resolves,
>   Cc, etc.) appearing more than once with the same value in a commit
>   message.
>
> - Self-review: a Reviewed-by, Tested-by, or Acked-by tag from the
>   same person as the patch author, which provides no independent
>   review value.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

ping

> ---
>  scripts/checkpatch.pl | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 03f35e75012c..a9326297d626 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1539,6 +1539,8 @@ sub process {
>
>         our $clean = 1;
>         my $signoff = 0;
> +       my $author_email = '';
> +       my %commit_trailers = ();
>         my $is_patch = 0;
>
>         my $in_header_lines = $file ? 0 : 1;
> @@ -1795,6 +1797,14 @@ sub process {
>                     ERROR("Author email address is mangled by the mailing list\n" . $herecurr);
>                 }
>
> +# Extract author email for trailer checks
> +               if ($in_header_lines && $line =~ /^(?:Author|From):\s*(.*)/) {
> +                       my $author_info = $1;
> +                       if ($author_info =~ /<([^>]+)>/) {
> +                               $author_email = $1;
> +                       }
> +               }
> +
>  #check the patch for a signoff:
>                 if ($line =~ /^\s*signed-off-by:/i) {
>                         # This is a signoff, if ugly, so do not double report.
> @@ -1811,6 +1821,28 @@ sub process {
>                         }
>                 }
>
> +# Check for duplicate trailers and self-review
> +               if (!$in_header_lines &&
> +                   $line =~ /^\s*([A-Z][a-zA-Z]*(?:-[a-zA-Z]+)*):\s*(.+)/) {
> +                       my $trailer_type = $1;
> +                       my $trailer_value = $2;
> +                       $trailer_value =~ s/\s+$//;
> +                       my $trailer_key = lc("$trailer_type: $trailer_value");
> +
> +                       if (exists $commit_trailers{$trailer_key}) {
> +                               WARN("Duplicate '$trailer_type' trailer\n" .
> +                                       $herecurr);
> +                       }
> +                       $commit_trailers{$trailer_key} = 1;
> +
> +                       if ($trailer_type =~ /^(?:Reviewed|Tested|Acked)-by$/ &&
> +                           $author_email ne '' &&
> +                           $trailer_value =~ /<\Q$author_email\E>/i) {
> +                               WARN("$trailer_type from the patch author\n" .
> +                                       $herecurr);
> +                       }
> +               }
> +
>  # Check SPDX-License-Identifier references a permitted license
>                 if (($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) &&
>                         $rawline !~ /^-/) {
> --
> 2.55.0
>
>