Re: cryptohash: missing locking functions for OpenSSL <= 1.0.2?

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