Re: [PATCH 3/3] send-email: use the three-arg form of open in recipients_cmd

2013-04-02 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 we can silence Perlcritique, even though we do not gain much safety
 by doing so.

Nit: it's perlcritic; critique is used to refer to the output of a critic.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] send-email: use the three-arg form of open in recipients_cmd

2013-03-31 Thread Junio C Hamano
From: Ramkumar Ramachandra artag...@gmail.com

Perlcritic does not want to see the trailing pipe in the two-args
form of open(), i.e.

open my $fh, $cmd \Q$file\E |;

If $cmd were a single-token command name, it would make a lot more
sense to use four-or-more-args form open FILEHANDLE,MODE,CMD,ARGS
to avoid shell from expanding metacharacters in $file, but we do
expect multi-word string in $to_cmd and $cc_cmd to be expanded by
the shell, so we cannot rewrite it to

open my $fh, -|, $cmd, $file;

for extra safety.  At least, by using this in the three-arg form:

open my $fh, -|, $cmd \Q$file\E;

we can silence Perlcritique, even though we do not gain much safety
by doing so.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-send-email.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 86dd593..fb92227 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1438,7 +1438,7 @@ sub recipients_cmd {
 
my $sanitized_sender = sanitize_address($sender);
my @addresses = ();
-   open my $fh, $cmd \Q$file\E |
+   open my $fh, -|, $cmd \Q$file\E
or die ($prefix) Could not execute '$cmd';
while (my $address = $fh) {
$address =~ s/^\s*//g;
-- 
1.8.2-441-g6e6f07b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] send-email: use the three-arg form of open in recipients_cmd

2013-03-31 Thread Jonathan Nieder
Junio C Hamano wrote:

   we cannot rewrite it to

   open my $fh, -|, $cmd, $file;

 for extra safety.  At least, by using this in the three-arg form:

   open my $fh, -|, $cmd \Q$file\E;

 we can silence Perlcritique, even though we do not gain much safety
 by doing so.

Yeah, I think this is the right thing to do.

This means that if some later code refactoring parses $tocmd once and
passes an array around, it would be easy to change this to

open my $fh, -|, @$cmd, $file;

and there would be no temptation to do something involving pasting
@$cmd back together into a single string.  Of course such a
refactoring is not very likely, but that kind of thing is a good
reason to prefer this style in general.

So for what it's worth, I like this patch.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html