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 :) Possibly. > > 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. Willy