linux-kernel - Re: [PATCH V2] git-send-email: Add ability to cc: any "bylines" in the commit message

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]

Date:   Wed, 31 Aug 2016 12:34:10 -0700
From:   Junio C Hamano <gitster@...ox.com>
To:     Joe Perches <joe@...ches.com>
Cc:     git@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] git-send-email: Add ability to cc: any "bylines" in the commit message

Joe Perches <joe@...ches.com> writes:

> Many commits have various forms of bylines similar to

A missing blank line (I can tweak while queuing).

>      "Acked-by: Name <address>" and "Reported-by: Name <address>"
>
> Add the ability to cc: bylines (e.g. Acked-by:) when using git send-email.
>
> This can be suppressed with --suppress-cc=bylines.
> ...
> @@ -307,8 +308,10 @@ Automating
>    patch body (commit message) except for self (use 'self' for that).
>  - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
>     for self (use 'self' for that).
> +- 'bylines' will avoid including anyone mentioned in any "<foo>-by:" lines
> +  in the patch header except for Signed-off-by.

<foo> feels a bit too informal but I don't think of a better
alternative, perhaps other than "*-by:".

> @@ -1545,7 +1545,7 @@ foreach my $t (@files) {
>  	# Now parse the message body
>  	while(<$fh>) {
>  		$message .=  $_;
> -		if (/^(Signed-off-by|Cc): (.*)$/i) {
> +		if (/^(Signed-off-by|Cc|[^\s]+[\w-]by): (.*)$/i) {

I thought you wanted

		if (/^(Signed-off-by|Cc|[\w-]+-by): (.*)$/i) {

instead to avoid "O_=:;fooby: Joe Perches <joe@...>"
>  			chomp;
>  			my ($what, $c) = ($1, $2);
>  			chomp $c;
> @@ -1555,6 +1555,12 @@ foreach my $t (@files) {
>  			} else {
>  				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
>  				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
> +				next if $suppress_cc{'bylines'} and $what !~ /Signed-off-by/i and $what =~ /by$/i;

Having to keep this /by$/i in sync with whatever definition of
bylines is will be error prone.  How about doing it in this way?

	# Now parse the message body
+	my $bypat = r/[\w-]+-by/;
	while (<$fh>) {
        	...
                if (/^(Signed-off-by|Cc|$bypat): (.*)$/i) {
                	...
                        	next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
+				next if $suppress_cc{'bylines'} and
+					$what !~ /^Signed-off-by/i and
+					$what =~ /^$bypat/i;

Other than that, looking good.