Incorrect comment for memset() on pgssHashKey?
Hi,
Commit 6b4d23feef introduces a toplevel field in pgssHashKey, which leads
padding. In pgss_store(), it comments that memset() is required when
pgssHashKey is without padding only.
@@ -1224,9 +1227,14 @@ pgss_store(const char *query, uint64 queryId,
query = CleanQuerytext(query, &query_location, &query_len);
/* Set up key for hashtable search */
+
+ /* memset() is required when pgssHashKey is without padding only */
+ memset(&key, 0, sizeof(pgssHashKey));
+
key.userid = GetUserId();
key.dbid = MyDatabaseId;
key.queryid = queryId;
+ key.toplevel = (exec_nested_level == 0);
/* Lookup the hash table entry with shared lock. */
LWLockAcquire(pgss->lock, LW_SHARED);
However, we need memset() only when pgssHashKey has padding, right?
--
Regrads,
Japin Li.
Attachments:
fix-incorrect-comment-for-pgss_store.patchtext/x-diffDownload
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 21bede29fe..f6ed87f1e6 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1257,7 +1257,7 @@ pgss_store(const char *query, uint64 queryId,
/* Set up key for hashtable search */
- /* memset() is required when pgssHashKey is without padding only */
+ /* memset() is required when pgssHashKey is with padding only */
memset(&key, 0, sizeof(pgssHashKey));
key.userid = GetUserId();
On 27/06/2023 07:32, Japin Li wrote:
Hi,
Commit 6b4d23feef introduces a toplevel field in pgssHashKey, which leads
padding. In pgss_store(), it comments that memset() is required when
pgssHashKey is without padding only.@@ -1224,9 +1227,14 @@ pgss_store(const char *query, uint64 queryId,
query = CleanQuerytext(query, &query_location, &query_len);/* Set up key for hashtable search */ + + /* memset() is required when pgssHashKey is without padding only */ + memset(&key, 0, sizeof(pgssHashKey)); + key.userid = GetUserId(); key.dbid = MyDatabaseId; key.queryid = queryId; + key.toplevel = (exec_nested_level == 0);/* Lookup the hash table entry with shared lock. */
LWLockAcquire(pgss->lock, LW_SHARED);However, we need memset() only when pgssHashKey has padding, right?
Yep. I changed the comment to just "clear padding". Thanks!
--
Heikki Linnakangas
Neon (https://neon.tech)