Document and check that PgStat_HashKey has no padding

This change is a tighter rework of 7d85d87f4d5c, which tried to improve
the code so as it would work should PgStat_HashKey gain new fields that
create padding bytes.  However, the previous change is proving to not be
enough as some code paths of pgstats do not pass PgStat_HashKey by
reference (valgrind would warn when padding is added to the structure,
through a new field).

Per discussion, let's document and check that PgStat_HashKey has no
padding rather than try to complicate the code of pgstats so as it is
able to work around that.

This removes a couple of memset(0) calls that should not be required.
While on it, this commit adds a static assertion checking that no
padding is introduced in the structure, by checking that the size of
PgStat_HashKey matches with the sum of the size of all its fields.

The object ID part of the hash key is already 8 bytes, which should be
plenty enough already.  A comment is added to discourage the addition of
new fields.

Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAA5RZ0t9omat+HVSakJXwTMWvhpYFcAZb41RPWKwrKFUgmAFBQ@mail.gmail.com

3 files changed, 19 insertions, 13 deletions

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index f8e91484e36..73c2ced3f4e 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -932,7 +932,7 @@ pgstat_clear_snapshot(void)
 void *
 pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
 {
-	PgStat_HashKey key;
+	PgStat_HashKey key = {0};
 	PgStat_EntryRef *entry_ref;
 	void	   *stats_data;
 	const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
@@ -943,9 +943,6 @@ pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
 
 	pgstat_prep_snapshot();
 
-	/* clear padding */
-	memset(&key, 0, sizeof(struct PgStat_HashKey));
-
 	key.kind = kind;
 	key.dboid = dboid;
 	key.objid = objid;
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 9dc3212f7dd..ca36fd247f6 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -456,14 +456,11 @@ PgStat_EntryRef *
 pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create,
 					 bool *created_entry)
 {
-	PgStat_HashKey key;
+	PgStat_HashKey key = {0};
 	PgStatShared_HashEntry *shhashent;
 	PgStatShared_Common *shheader = NULL;
 	PgStat_EntryRef *entry_ref;
 
-	/* clear padding */
-	memset(&key, 0, sizeof(struct PgStat_HashKey));
-
 	key.kind = kind;
 	key.dboid = dboid;
 	key.objid = objid;
@@ -988,13 +985,10 @@ pgstat_drop_database_and_contents(Oid dboid)
 bool
 pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
 {
-	PgStat_HashKey key;
+	PgStat_HashKey key = {0};
 	PgStatShared_HashEntry *shent;
 	bool		freed = true;
 
-	/* clear padding */
-	memset(&key, 0, sizeof(struct PgStat_HashKey));
-
 	key.kind = kind;
 	key.dboid = dboid;
 	key.objid = objid;
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 6cf00008f63..bf75ebcef31 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -48,7 +48,13 @@
  * PgStatShared_Common as the first element.
  */
 
-/* struct for shared statistics hash entry key. */
+/*
+ * Struct for shared statistics hash entry key.
+ *
+ * NB: We assume that this struct contains no padding.  Also, 8 bytes
+ * allocated for the object ID are good enough to ensure the uniqueness
+ * of the hash key, hence the addition of new fields is not recommended.
+ */
 typedef struct PgStat_HashKey
 {
 	PgStat_Kind kind;			/* statistics entry kind */
@@ -58,6 +64,15 @@ typedef struct PgStat_HashKey
 } PgStat_HashKey;
 
 /*
+ * PgStat_HashKey should not have any padding.  Checking that the structure
+ * size matches with the sum of each field is a check simple enough to
+ * enforce this policy.
+ */
+StaticAssertDecl((sizeof(PgStat_Kind) + sizeof(uint64) + sizeof(Oid)) ==
+				 sizeof(PgStat_HashKey),
+				 "PgStat_HashKey should have no padding");
+
+/*
  * Shared statistics hash entry. Doesn't itself contain any stats, but points
  * to them (with ->body). That allows the stats entries themselves to be of
  * variable size.