Re: Confusing #if nesting in hmac_openssl.c - Mailing list pgsql-hackers

From Daniel Gustafsson
Subject Re: Confusing #if nesting in hmac_openssl.c
Date
Msg-id F0A22A66-304F-4779-BF13-3E0B5AE6D72B@yesql.se
Whole thread Raw
In response to [MASSMAIL]Confusing #if nesting in hmac_openssl.c  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Confusing #if nesting in hmac_openssl.c
List pgsql-hackers
> On 2 Apr 2024, at 02:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> hmac_openssl.c:90:1: warning: unused function 'ResourceOwnerRememberHMAC' [-Wunused-function]
> hmac_openssl.c:95:1: warning: unused function 'ResourceOwnerForgetHMAC' [-Wunused-function]
>
> Looking at the code, this is all from commit e6bdfd970, and apparently
> batfish is our only animal that doesn't HAVE_HMAC_CTX_NEW.

Thanks for looking at this, it's been on my TODO for some time.  It's a warning
which only shows up when building against 1.0.2, the functions are present in
1.1.0 and onwards (while deprecated in 3.0).

> I don't think
> it is helpful to put the resource owner manipulations inside #ifdef
> HAVE_HMAC_CTX_NEW and HAVE_HMAC_CTX_FREE --- probably, it would never
> be the case that only one of those is defined,

Correct, no version of OpenSSL has only one of them defined.

> What do you think of rearranging it as attached?

+1 on this patch, it makes the #ifdef soup more readable.  We could go even
further and remove the HAVE_HMAC defines completely with USE_RESOWNER_FOR_HMAC
being set by autoconf/meson?  I've attached an untested sketch diff to
illustrate.

A related tangent.  If we assembled the data to calculate on ourselves rather
than rely on OpenSSL to do it with subsequent _update calls we could instead
use the simpler HMAC() API from OpenSSL.  That would remove the need for the
HMAC_CTX and resource owner tracking entirely and just have our pg_hmac_ctx.
Thats clearly not for this patch though, just thinking out loud that we set up
OpenSSL infrastructure that we don't really use.

--
Daniel Gustafsson


Attachment

pgsql-hackers by date:

Previous
From: Andy Fan
Date:
Subject: Re: [HACKERS] make async slave to wait for lsn to be replayed
Next
From: Alexander Korotkov
Date:
Subject: Re: [HACKERS] make async slave to wait for lsn to be replayed