[MASSMAIL]Confusing #if nesting in hmac_openssl.c - Mailing list pgsql-hackers

From Tom Lane
Subject [MASSMAIL]Confusing #if nesting in hmac_openssl.c
Date
Msg-id 1394271.1712016101@sss.pgh.pa.us
Whole thread Raw
Responses Re: Confusing #if nesting in hmac_openssl.c
List pgsql-hackers
I noticed that buildfarm member batfish has been complaining like
this for awhile:

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.  I tried to
understand the #if nesting and soon got very confused.  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, but it just seems
messy.  What do you think of rearranging it as attached?

            regards, tom lane

diff --git a/src/common/hmac_openssl.c b/src/common/hmac_openssl.c
index d10f7e5af7..4a62c8832f 100644
--- a/src/common/hmac_openssl.c
+++ b/src/common/hmac_openssl.c
@@ -41,6 +41,7 @@
  */
 #ifndef FRONTEND
 #ifdef HAVE_HMAC_CTX_NEW
+#define USE_RESOWNER_FOR_HMAC
 #define ALLOC(size) MemoryContextAlloc(TopMemoryContext, size)
 #else
 #define ALLOC(size) palloc(size)
@@ -67,13 +68,13 @@ struct pg_hmac_ctx
     pg_hmac_errno error;
     const char *errreason;
 
-#ifndef FRONTEND
+#ifdef USE_RESOWNER_FOR_HMAC
     ResourceOwner resowner;
 #endif
 };
 
 /* ResourceOwner callbacks to hold HMAC contexts */
-#ifndef FRONTEND
+#ifdef USE_RESOWNER_FOR_HMAC
 static void ResOwnerReleaseHMAC(Datum res);
 
 static const ResourceOwnerDesc hmac_resowner_desc =
@@ -138,10 +139,12 @@ pg_hmac_create(pg_cryptohash_type type)
      * previous runs.
      */
     ERR_clear_error();
-#ifdef HAVE_HMAC_CTX_NEW
-#ifndef FRONTEND
+
+#ifdef USE_RESOWNER_FOR_HMAC
     ResourceOwnerEnlarge(CurrentResourceOwner);
 #endif
+
+#ifdef HAVE_HMAC_CTX_NEW
     ctx->hmacctx = HMAC_CTX_new();
 #else
     ctx->hmacctx = ALLOC(sizeof(HMAC_CTX));
@@ -159,14 +162,14 @@ pg_hmac_create(pg_cryptohash_type type)
         return NULL;
     }
 
-#ifdef HAVE_HMAC_CTX_NEW
-#ifndef FRONTEND
+#ifndef HAVE_HMAC_CTX_NEW
+    memset(ctx->hmacctx, 0, sizeof(HMAC_CTX));
+#endif                            /* HAVE_HMAC_CTX_NEW */
+
+#ifdef USE_RESOWNER_FOR_HMAC
     ctx->resowner = CurrentResourceOwner;
     ResourceOwnerRememberHMAC(CurrentResourceOwner, ctx);
 #endif
-#else
-    memset(ctx->hmacctx, 0, sizeof(HMAC_CTX));
-#endif                            /* HAVE_HMAC_CTX_NEW */
 
     return ctx;
 }
@@ -327,15 +330,16 @@ pg_hmac_free(pg_hmac_ctx *ctx)
 
 #ifdef HAVE_HMAC_CTX_FREE
     HMAC_CTX_free(ctx->hmacctx);
-#ifndef FRONTEND
-    if (ctx->resowner)
-        ResourceOwnerForgetHMAC(ctx->resowner, ctx);
-#endif
 #else
     explicit_bzero(ctx->hmacctx, sizeof(HMAC_CTX));
     FREE(ctx->hmacctx);
 #endif
 
+#ifdef USE_RESOWNER_FOR_HMAC
+    if (ctx->resowner)
+        ResourceOwnerForgetHMAC(ctx->resowner, ctx);
+#endif
+
     explicit_bzero(ctx, sizeof(pg_hmac_ctx));
     FREE(ctx);
 }
@@ -375,7 +379,7 @@ pg_hmac_error(pg_hmac_ctx *ctx)
 
 /* ResourceOwner callbacks */
 
-#ifndef FRONTEND
+#ifdef USE_RESOWNER_FOR_HMAC
 static void
 ResOwnerReleaseHMAC(Datum res)
 {

pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: Table AM Interface Enhancements
Next
From: Tom Lane
Date:
Subject: Re: Why is parula failing?