On Wed, Feb 14, 2018 at 05:29:57PM +0100, Olivier Houchard wrote:
> Hi Willy,
> On Tue, Feb 13, 2018 at 08:05:44PM +0100, Willy Tarreau wrote:
> > Hi Olivier,
> > Such type of construct tends to scare me (probably because I'm not reading
> > the whole code). It means we're supposed to set an error by default unless
> > we pass by a specific path. I fear that we'll get future issues (possibly
> > as the result of further fixes for unrelated stuff).
> > 
> > Also I'm really not fond of the fact that the out_error label isn't used
> > anymore to set the error, but sometimes to set it, sometimes to clear it.
> > 
> What about what's attached, instead ?

I think it should work. Mateusz, care to give it a try to confirm ?
If OK, I'll merge it.

> > It's just a suggestion, it can be done by storing the result of
> > SSL_get_error() anywhere else, but definitely we need to make a clear
> > distinction between all these cases and for now the distinction between
> > the read0, completed connection and other errors is lost.
> > 
> Because we can't rely on ret == 0 being meaningful. The manpage for
> OpenSSL 1.1.1 states :
> Old documentation indicated a difference between 0 and -1, and that -1 was
> retryable. You should instead call SSL_get_error() to find out if it's
> retryable.

Interesting, indeed the doc and man page has changed between versions.
My man page explicitly mentions the fact that you need to use
SSL_get_error() to detect SSL_ERROR_SYSCALL, and then act depending
on what <ret> value was passed to it. Let's hope they only changed the
doc and not the semantics...

> And the switch would lead to more goto, as we need to break from outside the
> loop :)


> > I'm also seeing that the whole block could be rearranged so that ret <= 0
> > is handled first. That would remove some gotos and would leave the main
> > part after all error handling.
> > 
> I'm not sure I get that part. I don't mind one way or another, but I don't
> understand how it would remove gotos.

Note that I have not made the exercise.


Reply via email to