From: Junio C Hamano <gitster@pobox.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: git@vger.kernel.org, artagnon@gmail.com, mst@redhat.com
Subject: Re: [PATCH] send-email: don't call methods on undefined values
Date: Thu, 12 Sep 2013 23:09:45 -0700 [thread overview]
Message-ID: <xmqqd2odl0ba.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: 20130909224950.GA101065@vauxhall.crustytoothpaste.net
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> On Mon, Sep 09, 2013 at 09:45:10AM -0700, Junio C Hamano wrote:
>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> > --- a/git-send-email.perl
>> > +++ b/git-send-email.perl
>> > @@ -1234,7 +1234,7 @@ X-Mailer: git-send-email $gitversion
>> > if ($smtp->code == 220) {
>> > $smtp = Net::SMTP::SSL->start_SSL($smtp,
>> > ssl_verify_params())
>> > - or die "STARTTLS failed! ".$smtp->message;
>> > + or die "STARTTLS failed! ".IO::Socket::SSL::errstr();
>>
>> I agree that $smtp->message may be bogus at this point, but could
>> "require IO::Socket::SSL" have failed on us in ssl_verify_params?
>> In that degraded mode, we do not do SSL peer verification, but
>> otherwise we would still attempt to talk with the peer, and in such
>> a case, IO::Socket::SSL would not be available to us at this point,
>> no?
>
> Since Net::SMTP::SSL uses IO::Socket::SSL (in fact, it is an
> IO::Socket::SSL), we can be guaranteed that it is, in fact, available at
> this point. I guess strictly we don't need that require in
> IO::Socket::SSL since we'll already be guaranteed that it exists by the
> require of Net::SMTP::SSL.
That "require" came from around $gmane/230533, which was about
making sure the update to pacify newer Net::SMTP::SSL does not break
folks with older packages, I think.
I just didn't/don't know the history of Net::SMTP::SSL, and that was
where my comments came from.
As long as "Net::SMTP::SSL uses IO::Socket::SSL" has been true since
the ancient past, I agree that that 'require' of the latter does not
need to be guarded by an 'eval'; at that point, we are already in
the Net::SMTP::SSL codepath. And your patch I replied to should be
the right thing to do.
Thanks for clarifying.
prev parent reply other threads:[~2013-09-13 6:09 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-08 20:54 [PATCH] send-email: don't call methods on undefined values brian m. carlson
2013-09-09 16:45 ` Junio C Hamano
2013-09-09 22:49 ` brian m. carlson
2013-09-13 6:09 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqd2odl0ba.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=artagnon@gmail.com \
--cc=git@vger.kernel.org \
--cc=mst@redhat.com \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.