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. (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 :)
