> On 2 Apr 2024, at 02:01, Tom Lane <t...@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
hmac_context.diff
Description: Binary data
