On Wed, Oct 18, 2017 at 9:20 AM, Julien Rouhaud <rjuju...@gmail.com> wrote: > Sorry for replying so late, but I have a perhaps naive question about > the hashtable handling with this new version. > > IIUC, the shared hash table is now created with HASH_BLOBS instead of > HASH_FUNCTION, so since sizeof(pgssHashKey) != sizeof(uint32) the hash > table will use tag_hash() to compute the hash key. > > tag_hash() uses all the bits present in the given struct, so this can > be problematic if padding bits are not zeroed, which isn't garanted by > C standard for local variable. > > 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
