Re: [PATCH] send-email: improve recipients_cmd()

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2013, 19:18 UTC
Message-ID: <7vbo8yqxas.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1367101561-14671-1-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 8 quoted lines

> We don't need to quote the filename to pass to the command, we can use
> an array of all the arguments to pass to the command, which is safer,
> and more extensible.
>
> Commit a47eab0 (send-email: use the three-arg form of open in
> recipients_cmd) stated we couldn't pass $file directly, but in fact, we
> can, the multi-word string is passed as is, and we can pass an array
> too.

I think the comment is not about passing $file directly, but is about passing $cmd that could be multi-word string directly. The caller expects it be split into command and its earlier part of command line parameters, so that you can say

    $cmd = "cccmd --frotz --nitfol"

but the non-string form of open would not give you that, unless you rewrite it to

	open $fh, "-|", qw(sh -c), $cmd, @args

or something, no?

Show 23 quoted lines

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  git-send-email.perl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index bd13cc8..7880d12 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1452,11 +1452,11 @@ foreach my $t (@files) {
>  # Execute a command (e.g. $to_cmd) to get a list of email addresses
>  # and return a results array
>  sub recipients_cmd {
> -	my ($prefix, $what, $cmd, $file) = @_;
> +	my ($prefix, $what, $cmd, @args) = @_;
>  
>  	my $sanitized_sender = sanitize_address($sender);
>  	my @addresses = ();
> -	open my $fh, "-|", "$cmd \Q$file\E"
> +	open my $fh, "-|", $cmd, @args
>  	    or die "($prefix) Could not execute '$cmd'";
>  	while (my $address = <$fh>) {
>  		$address =~ s/^\s*//g;