unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: dick.r.chiang@gmail.com, Paul Eggert <eggert@cs.ucla.edu>
Cc: 49055@debbugs.gnu.org
Subject: bug#49055: 28.0.50; [PATCH] De-obfuscate gnutls_handshake loop
Date: Sat, 19 Jun 2021 20:51:51 +0300	[thread overview]
Message-ID: <83zgvlvj2g.fsf@gnu.org> (raw)
In-Reply-To: <875yy9wyg2.fsf@dick> (dick.r.chiang@gmail.com)

> From: dick.r.chiang@gmail.com
> Cc: 49055@debbugs.gnu.org
> Date: Sat, 19 Jun 2021 13:34:21 -0400
> 
> >   . the gnutls_error_is_fatal call is missing from the new code
> 
> Yes, and just as well since it's redundant with `emacs_gnutls_handle_error`.

Not really, no: emacs_gnutls_handle_error does more, so the code is
not equivalent.

> >   . the negative values of 'ret' (if they are significant) aren't
> >     tested anymore
> 
> This unchanged line 626 begs to differ.
> 
> while ((ret = gnutls_handshake (state)) < 0)

Only if we accept your removal of the inner loop.  Which I don't, at
least not yet; see below.

> >   . the condition of GNUTLS_E_INTERRUPTED is tested only once, and
> >     immediately causes the outer while-loop to be abandoned
> 
> Yes, as the commit before e87e6a2 did.  You do realize I hope that e87e6a2, in
> its desire to keep the loop going under GNUTLS_E_INTERRUPTED, almost
> certainly did not intend to call `gnutls_handshake` twice when
> GNUTLS_E_INTERRUPTED was not applicable.

Oh, yes, it did intend that: the changes are explicitly so that
interrupted calls don't fail because they were interrupted, but are
retried.

> > I'd love to see some rationale for these differences.
> 
> Your skepticism is a credit to your earnestness.  However, your expert
> scrutiny is better applied to misguided commits like e87e6a2 and d84d69d.

The thing is that I understand the changes in e87e6a2, and don't
consider them misguided.  They make Emacs more reliable in the rare
situations where the TLS handshake is interrupted.

What happens with your changes if during the TLS handshake you
repeatedly and quickly press C-g several times?

Paul, I'd appreciate your views on this, please.





      reply	other threads:[~2021-06-19 17:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16  1:11 bug#49055: 28.0.50; [PATCH] De-obfuscate gnutls_handshake loop dick.r.chiang
2021-06-19 13:19 ` Lars Ingebrigtsen
2021-06-19 13:37   ` Eli Zaretskii
2021-06-19 13:40     ` Lars Ingebrigtsen
2021-06-19 13:47       ` Eli Zaretskii
2021-06-19 17:34         ` dick.r.chiang
2021-06-19 17:51           ` Eli Zaretskii [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

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83zgvlvj2g.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=49055@debbugs.gnu.org \
    --cc=dick.r.chiang@gmail.com \
    --cc=eggert@cs.ucla.edu \
    /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.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).