Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

This one caught my attention:

diff --git a/contrib/pgcrypto/crypt-blowfish.c b/contrib/pgcrypto/crypt-blowfish.c
index a663852ccf..63fcef562d 100644
--- a/contrib/pgcrypto/crypt-blowfish.c
+++ b/contrib/pgcrypto/crypt-blowfish.c
@@ -750,7 +750,7 @@ _crypt_blowfish_rn(const char *key, const char *setting,
 /* Overwrite the most obvious sensitive data we have on the stack. Note
  * that this does not guarantee there's no sensitive data left on the
  * stack and/or in registers; I'm not aware of portable code that does. */
-    px_memset(&data, 0, sizeof(data));
+    px_memset(&data, 0, sizeof(struct data));
 
     return output;
 }

The curious thing here is that sizeof(data) is correct, because it
refers to a variable defined earlier in that function, whose type is an
anonymous struct declared there.  But I don't know what "struct data"
refers to, precisely because that struct is unnamed.  Am I misreading it?


Also:

diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index e1048e47ff..87be62f023 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -601,7 +601,7 @@ pgstathashindex(PG_FUNCTION_ARGS)
                  errmsg("cannot access temporary indexes of other sessions")));
 
     /* Get the information we need from the metapage. */
-    memset(&stats, 0, sizeof(stats));
+    memset(&stats, 0, sizeof(HashIndexStat));
     metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
     metap = HashPageGetMeta(BufferGetPage(metabuf));
     stats.version = metap->hashm_version;

I think the working theory here is that the original line is correct
now, and it continues to be correct if somebody edits the function and
makes variable 'stats' be of a different type.  But if you change the
sizeof() to use the type name, then there are two places that you need
to edit, and they are not necessarily close together; so it is correct
now and could become a bug in the future.  I don't think we're fully
consistent about this, but I think you're proposing to change it in the
opposite direction that we'd prefer.

For the case where the variable is a pointer, the developer could write
'sizeof(*variable)' instead of being forced to specify the type name,
for example (just a random one):

diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index a434cf93ef..e92c03686f 100644
--- a/contrib/bloom/blutils.c
+++ b/contrib/bloom/blutils.c
@@ -438,7 +438,7 @@ BloomFillMetapage(Relation index, Page metaPage)
      */
     BloomInitPage(metaPage, BLOOM_META);
     metadata = BloomPageGetMeta(metaPage);
-    memset(metadata, 0, sizeof(BloomMetaPageData));
+    memset(metadata, 0, sizeof(*metadata));
     metadata->magickNumber = BLOOM_MAGICK_NUMBER;
     metadata->opts = *opts;
     ((PageHeader) metaPage)->pd_lower += sizeof(BloomMetaPageData);

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/