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

From: Jacob Champion <pchampion(at)vmware(dot)com>
To: "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: cryptohash: missing locking functions for OpenSSL <= 1.0.2?
Date: 2021-03-08 18:06:32
Message-ID: 8200dc155937c1a7e5d9ea19cc65d95e89d0bc4d.camel@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ibrar Ahmed 2021-03-08 18:14:56 Re: partial heap only tuples
Previous Message Ibrar Ahmed 2021-03-08 17:33:24 Re: WIP: System Versioned Temporal Table