> On 30 Apr 2020, at 01:14, Michael Paquier <mich...@paquier.xyz> wrote: > > On Wed, Apr 29, 2020 at 01:57:49PM +0200, Daniel Gustafsson wrote: >> Working in the TLS corners of the backend, I found while re-reviewing and >> re-testing for the release that this patch actually was a small, but vital, >> brick shy of a load. The error handling is always invoked due to a set of >> missing braces. Going into the check will cause the context to be freed and >> be_tls_open_server error out. The tests added narrowly escapes it by not >> setting the max version in the final test, but I'm not sure it's worth >> changing >> that now as not setting a value is an interesting testcase too. Sorry for >> missing that at the time of reviewing. > > Good catch, fixed. We would still have keep around the SSL old > context if both bounds were set. Testing this case would mean one > extra full restart of the server, and I am not sure either if that's > worth the extra cost here.
Agreed. I don't think the cost is warranted given the low probability of new errors around here, so I think the commit as it stands is sufficient. Thanks. cheers ./daniel
