On Wed, 2021-03-03 at 15:30 +0900, Michael Paquier wrote:
> Extra eyes are welcome here, though I feel comfortable with the
> approach taken here.
I have one suggestion for the new logic:
> else
> {
> /*
> * In the non-SSL case, just remove the crypto callbacks.
> This code
> * path has no dependency on any pending SSL calls.
> */
> destroy_needed = true;
> }
> [...]
> if (destroy_needed && conn->crypto_loaded)
> {
> destroy_ssl_system();
> conn->crypto_loaded = false;
> }
I had to convince myself that this logic is correct -- we set
destroy_needed even if crypto is not enabled, but then check later to
make sure that crypto_loaded is true before doing anything. What would
you think about moving the conn->crypto_loaded check to the else
branch, so that destroy_needed is only set if we actually need it?
Either way, the patch looks good to me and behaves nicely in testing.
Thanks!
--Jacob
