hashmap: provide deallocation function names

hashmap_free(), hashmap_free_entries(), and hashmap_free_() have existed for a while, but aren't necessarily the clearest names, especially with hashmap_partial_clear() being added to the mix and lazy-initialization now being supported. Peff suggested we adopt the following names[1]: - hashmap_clear() - remove all entries and de-allocate any hashmap-specific data, but be ready for reuse - hashmap_clear_and_free() - ditto, but free the entries themselves - hashmap_partial_clear() - remove all entries but don't deallocate table - hashmap_partial_clear_and_free() - ditto, but free the entries This patch provides the new names and converts all existing callers over to the new naming scheme. [1] https://lore.kernel.org/git/20201030125059.GA3277724@coredump.intra.peff.net/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Nov 2, 2020 at 18:55 UTC 6da1a258142ac2422c8c57c54b92eaed3c86226e
22 files changed +63 -53
add-interactive.c
+1 -1
@@ -557,7 +557,7 @@ static int get_modified_files(struct repository *r,
557 if (ps)
558 clear_pathspec(&rev.prune_data);
559 }
560 - hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
560 + hashmap_clear_and_free(&s.file_map, struct pathname_entry, ent);
561 if (unmerged_count)
562 *unmerged_count = s.unmerged_count;
563 if (binary_count)
blame.c
+1 -1
@@ -435,7 +435,7 @@ static void get_fingerprint(struct fingerprint *result,
435
436 static void free_fingerprint(struct fingerprint *f)
437 {
438 - hashmap_free(&f->map);
438 + hashmap_clear(&f->map);
439 free(f->entries);
440 }
441
bloom.c
+1 -1
@@ -287,7 +287,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
287 }
288
289 cleanup:
290 - hashmap_free_entries(&pathmap, struct pathmap_hash_entry, entry);
290 + hashmap_clear_and_free(&pathmap, struct pathmap_hash_entry, entry);
291 } else {
292 for (i = 0; i < diff_queued_diff.nr; i++)
293 diff_free_filepair(diff_queued_diff.queue[i]);
builtin/fetch.c
+3 -3
@@ -393,7 +393,7 @@ static void find_non_local_tags(const struct ref *refs,
393 item = refname_hash_add(&remote_refs, ref->name, &ref->old_oid);
394 string_list_insert(&remote_refs_list, ref->name);
395 }
396 - hashmap_free_entries(&existing_refs, struct refname_hash_entry, ent);
396 + hashmap_clear_and_free(&existing_refs, struct refname_hash_entry, ent);
397
398 /*
399 * We may have a final lightweight tag that needs to be
@@ -428,7 +428,7 @@ static void find_non_local_tags(const struct ref *refs,
428 **tail = rm;
429 *tail = &rm->next;
430 }
431 - hashmap_free_entries(&remote_refs, struct refname_hash_entry, ent);
431 + hashmap_clear_and_free(&remote_refs, struct refname_hash_entry, ent);
432 string_list_clear(&remote_refs_list, 0);
433 oidset_clear(&fetch_oids);
434 }
@@ -573,7 +573,7 @@ static struct ref *get_ref_map(struct remote *remote,
573 }
574 }
575 if (existing_refs_populated)
576 - hashmap_free_entries(&existing_refs, struct refname_hash_entry, ent);
576 + hashmap_clear_and_free(&existing_refs, struct refname_hash_entry, ent);
577
578 return ref_map;
579 }
builtin/shortlog.c
+1 -1
@@ -220,7 +220,7 @@ static void strset_clear(struct strset *ss)
220 {
221 if (!ss->map.table)
222 return;
223 - hashmap_free_entries(&ss->map, struct strset_item, ent);
223 + hashmap_clear_and_free(&ss->map, struct strset_item, ent);
224 }
225
226 static void insert_records_from_trailers(struct shortlog *log,
config.c
+1 -1
@@ -1963,7 +1963,7 @@ void git_configset_clear(struct config_set *cs)
1963 free(entry->key);
1964 string_list_clear(&entry->value_list, 1);
1965 }
1966 - hashmap_free_entries(&cs->config_hash, struct config_set_element, ent);
1966 + hashmap_clear_and_free(&cs->config_hash, struct config_set_element, ent);
1967 cs->hash_initialized = 0;
1968 free(cs->list.items);
1969 cs->list.nr = 0;
diff.c
+2 -2
@@ -6289,9 +6289,9 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
6289 if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
6290 dim_moved_lines(o);
6291
6292 - hashmap_free_entries(&add_lines, struct moved_entry,
6292 + hashmap_clear_and_free(&add_lines, struct moved_entry,
6293 ent);
6294 - hashmap_free_entries(&del_lines, struct moved_entry,
6294 + hashmap_clear_and_free(&del_lines, struct moved_entry,
6295 ent);
6296 }
6297
diffcore-rename.c
+1 -1
@@ -407,7 +407,7 @@ static int find_exact_renames(struct diff_options *options)
407 renames += find_identical_files(&file_table, i, options);
408
409 /* Free the hash data structure and entries */
410 - hashmap_free_entries(&file_table, struct file_similarity, entry);
410 + hashmap_clear_and_free(&file_table, struct file_similarity, entry);
411
412 return renames;
413 }
dir.c
+4 -4
@@ -817,8 +817,8 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
817
818 clear_hashmaps:
819 warning(_("disabling cone pattern matching"));
820 - hashmap_free_entries(&pl->parent_hashmap, struct pattern_entry, ent);
821 - hashmap_free_entries(&pl->recursive_hashmap, struct pattern_entry, ent);
820 + hashmap_clear_and_free(&pl->parent_hashmap, struct pattern_entry, ent);
821 + hashmap_clear_and_free(&pl->recursive_hashmap, struct pattern_entry, ent);
822 pl->use_cone_patterns = 0;
823 }
824
@@ -921,8 +921,8 @@ void clear_pattern_list(struct pattern_list *pl)
921 free(pl->patterns[i]);
922 free(pl->patterns);
923 free(pl->filebuf);
924 - hashmap_free_entries(&pl->recursive_hashmap, struct pattern_entry, ent);
925 - hashmap_free_entries(&pl->parent_hashmap, struct pattern_entry, ent);
924 + hashmap_clear_and_free(&pl->recursive_hashmap, struct pattern_entry, ent);
925 + hashmap_clear_and_free(&pl->parent_hashmap, struct pattern_entry, ent);
926
927 memset(pl, 0, sizeof(*pl));
928 }
hashmap.c
+3 -3
@@ -183,7 +183,7 @@ static void free_individual_entries(struct hashmap *map, ssize_t entry_offset)
183 while ((e = hashmap_iter_next(&iter)))
184 /*
185 * like container_of, but using caller-calculated
186 - * offset (caller being hashmap_free_entries)
186 + * offset (caller being hashmap_clear_and_free)
187 */
188 free((char *)e - entry_offset);
189 }
@@ -199,11 +199,11 @@ void hashmap_partial_clear_(struct hashmap *map, ssize_t entry_offset)
199 map->private_size = 0;
200 }
201
202 -void hashmap_free_(struct hashmap *map, ssize_t entry_offset)
202 +void hashmap_clear_(struct hashmap *map, ssize_t entry_offset)
203 {
204 if (!map || !map->table)
205 return;
206 - if (entry_offset >= 0) /* called by hashmap_free_entries */
206 + if (entry_offset >= 0) /* called by hashmap_clear_and_free */
207 free_individual_entries(map, entry_offset);
208 free(map->table);
209 memset(map, 0, sizeof(*map));
hashmap.h
+27 -17
@@ -96,7 +96,7 @@
96 * }
97 *
98 * if (!strcmp("end", action)) {
99 - * hashmap_free_entries(&map, struct long2string, ent);
99 + * hashmap_clear_and_free(&map, struct long2string, ent);
100 * break;
101 * }
102 * }
@@ -237,7 +237,7 @@ void hashmap_init(struct hashmap *map,
237
238 /* internal functions for clearing or freeing hashmap */
239 void hashmap_partial_clear_(struct hashmap *map, ssize_t offset);
240 -void hashmap_free_(struct hashmap *map, ssize_t offset);
240 +void hashmap_clear_(struct hashmap *map, ssize_t offset);
241
242 /*
243 * Frees a hashmap structure and allocated memory for the table, but does not
@@ -253,40 +253,50 @@ void hashmap_free_(struct hashmap *map, ssize_t offset);
253 * free(e->somefield);
254 * free(e);
255 * }
256 - * hashmap_free(map);
256 + * hashmap_clear(map);
257 *
258 * instead of
259 *
260 * hashmap_for_each_entry(map, hashmap_iter, e, hashmap_entry_name) {
261 * free(e->somefield);
262 * }
263 - * hashmap_free_entries(map, struct my_entry_struct, hashmap_entry_name);
263 + * hashmap_clear_and_free(map, struct my_entry_struct, hashmap_entry_name);
264 *
265 * to avoid the implicit extra loop over the entries. However, if there are
266 * no special fields in your entry that need to be freed beyond the entry
267 * itself, it is probably simpler to avoid the explicit loop and just call
268 - * hashmap_free_entries().
268 + * hashmap_clear_and_free().
269 */
270 -#define hashmap_free(map) hashmap_free_(map, -1)
270 +#define hashmap_clear(map) hashmap_clear_(map, -1)
271
272 /*
273 - * Basically the same as calling hashmap_free() followed by hashmap_init(),
274 - * but doesn't incur the overhead of deallocating and reallocating
275 - * map->table; it leaves map->table allocated and the same size but zeroes
276 - * it out so it's ready for use again as an empty map. As with
277 - * hashmap_free(), you may need to free the entries yourself before calling
278 - * this function.
273 + * Similar to hashmap_clear(), except that the table is no deallocated; it
274 + * is merely zeroed out but left the same size as before. If the hashmap
275 + * will be reused, this avoids the overhead of deallocating and
276 + * reallocating map->table. As with hashmap_clear(), you may need to free
277 + * the entries yourself before calling this function.
278 */
279 #define hashmap_partial_clear(map) hashmap_partial_clear_(map, -1)
280
281 /*
283 - * Frees @map and all entries. @type is the struct type of the entry
284 - * where @member is the hashmap_entry struct used to associate with @map.
282 + * Similar to hashmap_clear() but also frees all entries. @type is the
283 + * struct type of the entry where @member is the hashmap_entry struct used
284 + * to associate with @map.
285 *
286 - * See usage note above hashmap_free().
286 + * See usage note above hashmap_clear().
287 */
288 -#define hashmap_free_entries(map, type, member) \
289 - hashmap_free_(map, offsetof(type, member));
288 +#define hashmap_clear_and_free(map, type, member) \
289 + hashmap_clear_(map, offsetof(type, member))
290 +
291 +/*
292 + * Similar to hashmap_partial_clear() but also frees all entries. @type is
293 + * the struct type of the entry where @member is the hashmap_entry struct
294 + * used to associate with @map.
295 + *
296 + * See usage note above hashmap_clear().
297 + */
298 +#define hashmap_partial_clear_and_free(map, type, member) \
299 + hashmap_partial_clear_(map, offsetof(type, member))
300
301 /* hashmap_entry functions */
302
merge-recursive.c
+3 -3
@@ -2651,7 +2651,7 @@ static struct string_list *get_renames(struct merge_options *opt,
2651 free(e->target_file);
2652 string_list_clear(&e->source_files, 0);
2653 }
2654 - hashmap_free_entries(&collisions, struct collision_entry, ent);
2654 + hashmap_clear_and_free(&collisions, struct collision_entry, ent);
2655 return renames;
2656 }
2657
@@ -2870,7 +2870,7 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs,
2870 strbuf_release(&e->new_dir);
2871 /* possible_new_dirs already cleared in get_directory_renames */
2872 }
2873 - hashmap_free_entries(dir_renames, struct dir_rename_entry, ent);
2873 + hashmap_clear_and_free(dir_renames, struct dir_rename_entry, ent);
2874 free(dir_renames);
2875
2876 free(pairs->queue);
@@ -3497,7 +3497,7 @@ static int merge_trees_internal(struct merge_options *opt,
3497 string_list_clear(entries, 1);
3498 free(entries);
3499
3500 - hashmap_free_entries(&opt->priv->current_file_dir_set,
3500 + hashmap_clear_and_free(&opt->priv->current_file_dir_set,
3501 struct path_hashmap_entry, e);
3502
3503 if (clean < 0) {
name-hash.c
+2 -2
@@ -726,6 +726,6 @@ void free_name_hash(struct index_state *istate)
726 return;
727 istate->name_hash_initialized = 0;
728
729 - hashmap_free(&istate->name_hash);
730 - hashmap_free_entries(&istate->dir_hash, struct dir_entry, ent);
729 + hashmap_clear(&istate->name_hash);
730 + hashmap_clear_and_free(&istate->dir_hash, struct dir_entry, ent);
731 }
object.c
+1 -1
@@ -532,7 +532,7 @@ void raw_object_store_clear(struct raw_object_store *o)
532 close_object_store(o);
533 o->packed_git = NULL;
534
535 - hashmap_free(&o->pack_map);
535 + hashmap_clear(&o->pack_map);
536 }
537
538 void parsed_object_pool_clear(struct parsed_object_pool *o)
oidmap.c
+1 -1
@@ -27,7 +27,7 @@ void oidmap_free(struct oidmap *map, int free_entries)
27 return;
28
29 /* TODO: make oidmap itself not depend on struct layouts */
30 - hashmap_free_(&map->map, free_entries ? 0 : -1);
30 + hashmap_clear_(&map->map, free_entries ? 0 : -1);
31 }
32
33 void *oidmap_get(const struct oidmap *map, const struct object_id *key)
patch-ids.c
+1 -1
@@ -71,7 +71,7 @@ int init_patch_ids(struct repository *r, struct patch_ids *ids)
71
72 int free_patch_ids(struct patch_ids *ids)
73 {
74 - hashmap_free_entries(&ids->patches, struct patch_id, ent);
74 + hashmap_clear_and_free(&ids->patches, struct patch_id, ent);
75 return 0;
76 }
77
range-diff.c
+1 -1
@@ -266,7 +266,7 @@ static void find_exact_matches(struct string_list *a, struct string_list *b)
266 }
267 }
268
269 - hashmap_free(&map);
269 + hashmap_clear(&map);
270 }
271
272 static void diffsize_consume(void *data, char *line, unsigned long len)
ref-filter.c
+1 -1
@@ -2222,7 +2222,7 @@ void ref_array_clear(struct ref_array *array)
2222 used_atom_cnt = 0;
2223
2224 if (ref_to_worktree_map.worktrees) {
2225 - hashmap_free_entries(&(ref_to_worktree_map.map),
2225 + hashmap_clear_and_free(&(ref_to_worktree_map.map),
2226 struct ref_to_worktree_entry, ent);
2227 free_worktrees(ref_to_worktree_map.worktrees);
2228 ref_to_worktree_map.worktrees = NULL;
revision.c
+1 -1
@@ -139,7 +139,7 @@ static void paths_and_oids_clear(struct hashmap *map)
139 free(entry->path);
140 }
141
142 - hashmap_free_entries(map, struct path_and_oids_entry, ent);
142 + hashmap_clear_and_free(map, struct path_and_oids_entry, ent);
143 }
144
145 static void paths_and_oids_insert(struct hashmap *map,
sequencer.c
+2 -2
@@ -5058,7 +5058,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
5058
5059 oidmap_free(&commit2todo, 1);
5060 oidmap_free(&state.commit2label, 1);
5061 - hashmap_free_entries(&state.labels, struct labels_entry, entry);
5061 + hashmap_clear_and_free(&state.labels, struct labels_entry, entry);
5062 strbuf_release(&state.buf);
5063
5064 return 0;
@@ -5577,7 +5577,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
5577 for (i = 0; i < todo_list->nr; i++)
5578 free(subjects[i]);
5579 free(subjects);
5580 - hashmap_free_entries(&subject2item, struct subject2item_entry, entry);
5580 + hashmap_clear_and_free(&subject2item, struct subject2item_entry, entry);
5581
5582 clear_commit_todo_item(&commit_todo);
5583
submodule-config.c
+2 -2
@@ -103,8 +103,8 @@ static void submodule_cache_clear(struct submodule_cache *cache)
103 ent /* member name */)
104 free_one_config(entry);
105
106 - hashmap_free_entries(&cache->for_path, struct submodule_entry, ent);
107 - hashmap_free_entries(&cache->for_name, struct submodule_entry, ent);
106 + hashmap_clear_and_free(&cache->for_path, struct submodule_entry, ent);
107 + hashmap_clear_and_free(&cache->for_name, struct submodule_entry, ent);
108 cache->initialized = 0;
109 cache->gitmodules_read = 0;
110 }
t/helper/test-hashmap.c
+3 -3
@@ -110,7 +110,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)
110 hashmap_add(&map, &entries[i]->ent);
111 }
112
113 - hashmap_free(&map);
113 + hashmap_clear(&map);
114 }
115 } else {
116 /* test map lookups */
@@ -130,7 +130,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)
130 }
131 }
132
133 - hashmap_free(&map);
133 + hashmap_clear(&map);
134 }
135 }
136
@@ -262,6 +262,6 @@ int cmd__hashmap(int argc, const char **argv)
262 }
263
264 strbuf_release(&line);
265 - hashmap_free_entries(&map, struct test_entry, ent);
265 + hashmap_clear_and_free(&map, struct test_entry, ent);
266 return 0;
267 }