> On 30 Apr 2020, at 01:14, Michael Paquier <michael@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
