From: Junio C Hamano Date: Wed, 27 Feb 2013 15:54:46 GMT Subject: Re: [PATCHv4 6/6] git-send-email: use git credential to obtain password Message-ID: <7vehg1kb09.fsf@alter.siamese.dyndns.org> In-Reply-To: Matthieu Moy writes: > Michal Nazarewicz writes: > >> + $auth = Git::credential({ >> + 'protocol' => 'smtp', >> + 'host' => join(':', $smtp_server, $smtp_server_port), > > At this point, $smtp_server_port is not always defined. I just tested > and got > > Use of uninitialized value $smtp_server_port in join or string at > git-send-email line 1077. > > Other than that, the whole series looks good. Given that there is another place that conditionally append ":$port" to the host string, I think we should follow suit here. Perhaps like the attached diff? Thanks for a review. git-send-email.perl | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 76bbfc3..c3501d9 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1045,6 +1045,14 @@ 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. @@ -1065,7 +1073,7 @@ sub smtp_auth_maybe { # reject credentials. $auth = Git::credential({ 'protocol' => 'smtp', - 'host' => join(':', $smtp_server, $smtp_server_port), + '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. @@ -1188,9 +1196,7 @@ sub send_message { 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) {