"Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > From: Elijah Newren <newren@xxxxxxxxx> > > In the series merged at bf0a430f70b5 (Merge branch 'en/strmap', > 2020-11-21), strmap was built on top of hashmap and hashmap was extended > in a few ways to support strmap and be more generally useful. One of > the extensions was that hashmap_partial_clear() was introduced to allow > reuse of the hashmap without freeing the table. Peff believed that it > also made sense to introduce a hashmap_clear() which freed everything > while allowing reuse. > > I added hashmap_clear(), but in doing so, overlooked the fact that for > a hashmap to be reusable, it needs a defined cmpfn and data (the > HASHMAP_INIT macro requires these fields as parameters, for example). > So, if we want the hashmap to be reusable, we shouldn't zero out those > fields. We probably also shouldn't zero out do_count_items. (We could > zero out grow_at and shrink_at, but whether we zero those or not is > irrelevant as they'll be automatically updated whenever a new entry is > inserted.) > > Since clearing is associated with freeing map->table, and the only thing > required for consistency after freeing map->table is zeroing tablesize > and private_size, let's only zero those fields out. Makes sense. Thanks for finding and fixing. I do not think we want to patch all the way down to Git 2.30, ... > diff --git a/hashmap.c b/hashmap.c > index ee45ef00852..a711377853f 100644 > --- a/hashmap.c > +++ b/hashmap.c > @@ -205,8 +205,9 @@ void hashmap_clear_(struct hashmap *map, ssize_t entry_offset) > return; > if (entry_offset >= 0) /* called by hashmap_clear_and_free */ > free_individual_entries(map, entry_offset); > - free(map->table); > - memset(map, 0, sizeof(*map)); > + FREE_AND_NULL(map->table); > + map->tablesize = 0; > + map->private_size = 0; > } > > struct hashmap_entry *hashmap_get(const struct hashmap *map, > > base-commit: f65182a99e545d2f2bc22e6c1c2da192133b16a3 ... but this part of the code has been fairly quiet and the patch applies very cleanly. So I'll apply on top of bf0a430f7 and merge the result---anybody maintaining Git for their LTS distro can then merge it to their favorite ancient maintenance track ;-) Thanks.
