On Thu, Oct 19, 2017 at 1:08 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Thu, Oct 19, 2017 at 4:12 AM, Robert Haas <robertmh...@gmail.com> wrote: >> On Wed, Oct 18, 2017 at 9:20 AM, Julien Rouhaud <rjuju...@gmail.com> wrote: >>> WIth current pgssHashKey definition, there shouldn't be padding bits, >>> so it should be safe. But I wonder if adding an explicit memset() of >>> the key in pgss_store() could avoid extension authors to have >>> duplicate entries if they rely on this code, or prevent future issue >>> in the unlikely case of adding other fields to pgssHashKey. >> >> I guess we should probably add additional comment to the definition of >> pgssHashKey warning of the danger. I'm OK with adding a memset if >> somebody can promise me it will get optimized away by all reasonably >> commonly-used compilers, but I'm not that keen on adding more cycles >> to protect against a hypothetical danger. > > A comment is an adapted answer for me too.
I agree, and I'm perfectly fine with adding a comment around pgssHashKey. PFA a patch to warn about the danger.
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index b04b4d6ce1..829ee69f51 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -124,7 +124,10 @@ typedef enum pgssVersion /* * Hashtable key that defines the identity of a hashtable entry. We separate - * queries by user and by database even if they are otherwise identical. + * queries by user and by database even if they are otherwise identical. Be + * careful when adding new fields, tag_hash() is used to compute the hash key, + * so we rely on the fact that no padding bit is present in this structure. + * Otherwise, we'd have to zero the key variable in pgss_store. */ typedef struct pgssHashKey {
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers