From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 5 Apr 2013 21:14:48 +0000 (-0700)
Subject: Merge branch 'rr/send-email-perl-critique'
X-Git-Tag: v1.8.3-rc0~113
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/cb660275789973aa56128b3fbbda4bdff270572a?ds=inline;hp=-c

Merge branch 'rr/send-email-perl-critique'

Update "git send-email" for issues noticed by PerlCritic.

* rr/send-email-perl-critique:
send-email: use the three-arg form of open in recipients_cmd
send-email: drop misleading function prototype
send-email: use "return;" not "return undef;" on error codepaths
---

cb660275789973aa56128b3fbbda4bdff270572a
diff --combined git-send-email.perl
index c3501d987e,70cad15ec4..7297472818
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@@ -512,8 -512,9 +512,9 @@@ if (@alias_files and $aliasfiletype an
  
  ($sender) = expand_aliases($sender) if defined $sender;
  
- # returns 1 if the conflict must be solved using it as a format-patch argument
- sub check_file_rev_conflict($) {
+ # is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
+ # $f is a revision list specification to be passed to format-patch.
+ sub is_format_patch_arg {
  	return unless $repo;
  	my $f = shift;
  	try {
@@@ -529,6 -530,7 +530,7 @@@ to produce patches for.  Please disambi
      * Giving --format-patch option if you mean a range.
  EOF
  	} catch Git::Error::Command with {
+ 		# Not a valid revision.  Treat it as a filename.
  		return 0;
  	}
  }
@@@ -540,14 -542,14 +542,14 @@@ while (defined(my $f = shift @ARGV)) 
  	if ($f eq "--") {
  		push @rev_list_opts, "--", @ARGV;
  		@ARGV = ();
- 	} elsif (-d $f and !check_file_rev_conflict($f)) {
+ 	} elsif (-d $f and !is_format_patch_arg($f)) {
  		opendir my $dh, $f
  			or die "Failed to opendir $f: $!";
  
  		push @files, grep { -f $_ } map { catfile($f, $_) }
  				sort readdir $dh;
  		closedir $dh;
- 	} elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) {
+ 	} elsif ((-f $f or -p $f) and !is_format_patch_arg($f)) {
  		push @files, $f;
  	} else {
  		push @rev_list_opts, $f;
@@@ -711,7 -713,7 +713,7 @@@ sub ask 
  			}
  		}
  	}
- 	return undef;
+ 	return;
  }
  
  my %broken_encoding;
@@@ -833,7 -835,7 +835,7 @@@ sub extract_valid_address 
  	# less robust/correct than the monster regexp in Email::Valid,
  	# but still does a 99% job, and one less dependency
  	return $1 if $address =~ /($local_part_regexp\@$domain_regexp)/;
- 	return undef;
+ 	return;
  }
  
  sub extract_valid_address_or_die {
@@@ -1045,47 -1047,6 +1047,47 @@@ sub maildomain 
  	return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
  }
  
 +sub smtp_host_string {
 +	if (defined $smtp_server_port) {
 +		return "$smtp_server:$smtp_server_port";
 +	} else {
 +		return $smtp_server;
 +	}
 +}
 +
 +# Returns 1 if authentication succeeded or was not necessary
 +# (smtp_user was not specified), and 0 otherwise.
 +
 +sub smtp_auth_maybe {
 +	if (!defined $smtp_authuser || $auth) {
 +		return 1;
 +	}
 +
 +	# Workaround AUTH PLAIN/LOGIN interaction defect
 +	# with Authen::SASL::Cyrus
 +	eval {
 +		require Authen::SASL;
 +		Authen::SASL->import(qw(Perl));
 +	};
 +
 +	# TODO: Authentication may fail not because credentials were
 +	# invalid but due to other reasons, in which we should not
 +	# reject credentials.
 +	$auth = Git::credential({
 +		'protocol' => 'smtp',
 +		'host' => smtp_host_string(),
 +		'username' => $smtp_authuser,
 +		# if there's no password, "git credential fill" will
 +		# give us one, otherwise it'll just pass this one.
 +		'password' => $smtp_authpass
 +	}, sub {
 +		my $cred = shift;
 +		return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
 +	});
 +
 +	return $auth;
 +}
 +
  # Returns 1 if the message was sent, and 0 otherwise.
  # In actuality, the whole program dies when there
  # is an error sending a message.
@@@ -1196,7 -1157,9 +1198,7 @@@ X-Mailer: git-send-email $gitversio
  		else {
  			require Net::SMTP;
  			$smtp_domain ||= maildomain();
 -			$smtp ||= Net::SMTP->new((defined $smtp_server_port)
 -						 ? "$smtp_server:$smtp_server_port"
 -						 : $smtp_server,
 +			$smtp ||= Net::SMTP->new(smtp_host_string(),
  						 Hello => $smtp_domain,
  						 Debug => $debug_net_smtp);
  			if ($smtp_encryption eq 'tls' && $smtp) {
@@@ -1224,7 -1187,31 +1226,7 @@@
  			    defined $smtp_server_port ? " port=$smtp_server_port" : "";
  		}
  
 -		if (defined $smtp_authuser) {
 -			# Workaround AUTH PLAIN/LOGIN interaction defect
 -			# with Authen::SASL::Cyrus
 -			eval {
 -				require Authen::SASL;
 -				Authen::SASL->import(qw(Perl));
 -			};
 -
 -			if (!defined $smtp_authpass) {
 -
 -				system "stty -echo";
 -
 -				do {
 -					print "Password: ";
 -					$_ = <STDIN>;
 -					print "\n";
 -				} while (!defined $_);
 -
 -				chomp($smtp_authpass = $_);
 -
 -				system "stty echo";
 -			}
 -
 -			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
 -		}
 +		smtp_auth_maybe or die $smtp->message;
  
  		$smtp->mail( $raw_from ) or die $smtp->message;
  		$smtp->to( @recipients ) or die $smtp->message;
@@@ -1453,7 -1440,7 +1455,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;
@@@ -1499,7 -1486,7 +1501,7 @@@ sub validate_patch 
  			return "$.: patch contains a line longer than 998 characters";
  		}
  	}
- 	return undef;
+ 	return;
  }
  
  sub file_has_nonascii {