Hi Eric
On 27/08/2019 10:49, Eric Wong wrote:
Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote:
Hi Eric,
On Mon, 26 Aug 2019, Eric Wong wrote:
By renaming the "hash" field to "_hash", it's easy to spot
improper initialization of hashmap_entry structs which
can leave "hashmap_entry.next" uninitialized.
Would you mind elaborating a bit? This explanation does not enlighten
me, sadly, all I see is that it makes it (slightly) harder for me to
maintain Git for Windows' patches on top of `pu`, as the FSCache patches
access that field directly (so even if they rebase cleanly, the build
breaks).
I renamed it to intentionally break my build.
That way I could easily spot if there were any other improper
initializations of the .hash field. It's fine to revert,
actually, it could be more of a "showing my work" patch.
I'm still a bit confused as the changed initializations already used
hashmap_entry_init() and so presumably were already initializing
hashmap_entry.next correctly. Is there a way to get 'make coccicheck'
detect incorrect initializations, this renaming wont prevent bad code
being added in the future.
Best Wishes
Phillip
(AFAIK, it's a pretty common practice, but maybe not here :x)
I've also pondered adding a
"hashmap_entry_hash(const struct hashmap_entry *)"
accessor method for reading the field value (but not setting
it), but it's a bit verbose...
I'm also wondering where/if hashmap offers a real benefit over
khash nowadays; the latter ought to have better locality.
Would like benchmark at some point in the future;
but safety fixes first :)
