> 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