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: >> 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.
A comment is an adapted answer for me too. There is no guarantee that memset improvements will get committed. They will likely be, but making a hard promise is difficult. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers