On Thu, Oct 19, 2017 at 1:08 AM, Michael Paquier
<[email protected]> wrote:
> On Thu, Oct 19, 2017 at 4:12 AM, Robert Haas <[email protected]> wrote:
>> On Wed, Oct 18, 2017 at 9:20 AM, Julien Rouhaud <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
