Re: Confusing #if nesting in hmac_openssl.c

> On 2 Apr 2024, at 02:01, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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