613491 - Wonky code in MultinameHashtable

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Tommy Reilly]

int bitMask = numQuads - 1;
        // Note: Mask off MSB to avoid negative indices.  Mask off bottom
        // 3 bits because it doesn't contribute to hash.  Quad it
        // because names, namespaces, and values are stored adjacently.
        unsigned i = ((0x7FFFFFF8 & (uintptr_t)mnameName)>>3) & bitMask;

has always seemed wonky to be and probably time to consider fixing it: 

(1) "Mask off MSB to avoid negative indices", but uintptr_t is unsigned, so
this is pointless
(2) "Mask off bottom 3 bits" is a waste since we are shifting those bits out
anyway, via >>3
(3) "Quad it because names, namespaces, and values are stored adjacently" is
nonsensical and I think applies to a long-dead implementation

Can anyone explain why this is just as good:

        uintptr_t bitMask = numQuads - 1;
        uintptr_t i = (uintptr_t(mnameName)>>3) & bitMask;

Comment 1

[Tommy Reilly]

Attribution: that first comment is authored by stejohns

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.