From e010a41216efd85eacc4c0711ea405de4fb20526 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:24 +0000 Subject: [PATCH 01/20] diff: use hashmap_entry_init on moved_entry.ent Otherwise, the hashmap_entry.next field appears to remain uninitialized, which can lead to problems when add_lines_to_move_detection calls hashmap_add. I found this through manual inspection when converting hashmap_add callers to take "struct hashmap_entry *". Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- diff.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/diff.c b/diff.c index efe42b341a..02491ee684 100644 --- a/diff.c +++ b/diff.c @@ -964,8 +964,9 @@ static struct moved_entry *prepare_entry(struct diff_options *o, struct moved_entry *ret = xmalloc(sizeof(*ret)); struct emitted_diff_symbol *l = &o->emitted_symbols->buf[line_no]; unsigned flags = o->color_moved_ws_handling & XDF_WHITESPACE_FLAGS; + unsigned int hash = xdiff_hash_string(l->line, l->len, flags); - ret->ent.hash = xdiff_hash_string(l->line, l->len, flags); + hashmap_entry_init(&ret->ent, hash); ret->es = l; ret->next_line = NULL; From 12878c83516e3b82f31a20f1b5431b7ff607a8db Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:25 +0000 Subject: [PATCH 02/20] coccicheck: detect hashmap_entry.hash assignment Assigning hashmap_entry.hash manually leaves hashmap_entry.next uninitialized, which can be dangerous once the hashmap_entry is inserted into a hashmap. Detect those assignments and use hashmap_entry_init, instead. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- contrib/coccinelle/hashmap.cocci | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 contrib/coccinelle/hashmap.cocci diff --git a/contrib/coccinelle/hashmap.cocci b/contrib/coccinelle/hashmap.cocci new file mode 100644 index 0000000000..d69e120ccf --- /dev/null +++ b/contrib/coccinelle/hashmap.cocci @@ -0,0 +1,16 @@ +@ hashmap_entry_init_usage @ +expression E; +struct hashmap_entry HME; +@@ +- HME.hash = E; ++ hashmap_entry_init(&HME, E); + +@@ +identifier f !~ "^hashmap_entry_init$"; +expression E; +struct hashmap_entry *HMEP; +@@ + f(...) {<... +- HMEP->hash = E; ++ hashmap_entry_init(HMEP, E); + ...>} From d0a48a0a1d0df49af2e5fd6a80b0d84776c285aa Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:26 +0000 Subject: [PATCH 03/20] packfile: use hashmap_entry in delta_base_cache_entry This hashmap_entry_init function is intended to take a hashmap_entry struct pointer, not a hashmap struct pointer. This was not noticed because hashmap_entry_init takes a "void *" arg instead of "struct hashmap_entry *", and the hashmap struct is larger and can be cast into a hashmap_entry struct without data corruption. This has the beneficial side effect of reducing the size of a delta_base_cache_entry from 104 bytes to 72 bytes on 64-bit systems. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- packfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packfile.c b/packfile.c index fc43a6c52c..37fe0b73a6 100644 --- a/packfile.c +++ b/packfile.c @@ -1361,7 +1361,7 @@ struct delta_base_cache_key { }; struct delta_base_cache_entry { - struct hashmap hash; + struct hashmap_entry ent; struct delta_base_cache_key key; struct list_head lru; void *data; From d22245a2e360d2e708ca37169be8eb5a5899b98d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:27 +0000 Subject: [PATCH 04/20] hashmap_entry_init takes "struct hashmap_entry *" C compilers do type checking to make life easier for us. So rely on that and update all hashmap_entry_init callers to take "struct hashmap_entry *" to avoid future bugs while improving safety and readability. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 4 ++-- blame.c | 2 +- builtin/describe.c | 2 +- builtin/difftool.c | 6 +++--- builtin/fast-export.c | 2 +- builtin/fetch.c | 2 +- config.c | 4 ++-- diffcore-rename.c | 2 +- hashmap.c | 4 ++-- hashmap.h | 12 ++++++------ merge-recursive.c | 13 +++++++------ name-hash.c | 10 +++++----- packfile.c | 2 +- patch-ids.c | 2 +- range-diff.c | 4 ++-- ref-filter.c | 3 ++- refs.c | 2 +- remote.c | 2 +- revision.c | 4 ++-- sequencer.c | 5 +++-- sub-process.c | 4 ++-- submodule-config.c | 10 +++++----- t/helper/test-hashmap.c | 6 +++--- 23 files changed, 55 insertions(+), 52 deletions(-) diff --git a/attr.c b/attr.c index 93dc16b59c..a8be7a7b4f 100644 --- a/attr.c +++ b/attr.c @@ -98,7 +98,7 @@ static void *attr_hashmap_get(struct attr_hashmap *map, if (!map->map.tablesize) attr_hashmap_init(map); - hashmap_entry_init(&k, memhash(key, keylen)); + hashmap_entry_init(&k.ent, memhash(key, keylen)); k.key = key; k.keylen = keylen; e = hashmap_get(&map->map, &k, NULL); @@ -117,7 +117,7 @@ static void attr_hashmap_add(struct attr_hashmap *map, attr_hashmap_init(map); e = xmalloc(sizeof(struct attr_hash_entry)); - hashmap_entry_init(e, memhash(key, keylen)); + hashmap_entry_init(&e->ent, memhash(key, keylen)); e->key = key; e->keylen = keylen; e->value = value; diff --git a/blame.c b/blame.c index 36a2e7ef11..46059410cd 100644 --- a/blame.c +++ b/blame.c @@ -417,7 +417,7 @@ static void get_fingerprint(struct fingerprint *result, /* Ignore whitespace pairs */ if (hash == 0) continue; - hashmap_entry_init(entry, hash); + hashmap_entry_init(&entry->entry, hash); found_entry = hashmap_get(&result->map, entry, NULL); if (found_entry) { diff --git a/builtin/describe.c b/builtin/describe.c index 200154297d..596ddf89a5 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -123,7 +123,7 @@ static void add_to_known_names(const char *path, if (!e) { e = xmalloc(sizeof(struct commit_name)); oidcpy(&e->peeled, peeled); - hashmap_entry_init(e, oidhash(peeled)); + hashmap_entry_init(&e->entry, oidhash(peeled)); hashmap_add(&names, e); e->path = NULL; } diff --git a/builtin/difftool.c b/builtin/difftool.c index 16eb8b70ea..98ffc04c61 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -161,7 +161,7 @@ static void add_left_or_right(struct hashmap *map, const char *path, struct pair_entry *e, *existing; FLEX_ALLOC_STR(e, path, path); - hashmap_entry_init(e, strhash(path)); + hashmap_entry_init(&e->entry, strhash(path)); existing = hashmap_get(map, e, NULL); if (existing) { free(e); @@ -234,7 +234,7 @@ static void changed_files(struct hashmap *result, const char *index_path, while (!strbuf_getline_nul(&buf, fp)) { struct path_entry *entry; FLEX_ALLOC_STR(entry, path, buf.buf); - hashmap_entry_init(entry, strhash(buf.buf)); + hashmap_entry_init(&entry->entry, strhash(buf.buf)); hashmap_add(result, entry); } fclose(fp); @@ -461,7 +461,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, /* Avoid duplicate working_tree entries */ FLEX_ALLOC_STR(entry, path, dst_path); - hashmap_entry_init(entry, strhash(dst_path)); + hashmap_entry_init(&entry->entry, strhash(dst_path)); if (hashmap_get(&working_tree_dups, entry, NULL)) { free(entry); continue; diff --git a/builtin/fast-export.c b/builtin/fast-export.c index f541f55d33..287dbd24a3 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -148,7 +148,7 @@ static const void *anonymize_mem(struct hashmap *map, if (!map->cmpfn) hashmap_init(map, anonymized_entry_cmp, NULL, 0); - hashmap_entry_init(&key, memhash(orig, *len)); + hashmap_entry_init(&key.hash, memhash(orig, *len)); key.orig = orig; key.orig_len = *len; ret = hashmap_get(map, &key, NULL); diff --git a/builtin/fetch.c b/builtin/fetch.c index 717dd14e89..b7d70eee70 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -276,7 +276,7 @@ static struct refname_hash_entry *refname_hash_add(struct hashmap *map, size_t len = strlen(refname); FLEX_ALLOC_MEM(ent, refname, refname, len); - hashmap_entry_init(ent, strhash(refname)); + hashmap_entry_init(&ent->ent, strhash(refname)); oidcpy(&ent->oid, oid); hashmap_add(map, ent); return ent; diff --git a/config.c b/config.c index 3900e4947b..08d866e7de 100644 --- a/config.c +++ b/config.c @@ -1861,7 +1861,7 @@ static struct config_set_element *configset_find_element(struct config_set *cs, if (git_config_parse_key(key, &normalized_key, NULL)) return NULL; - hashmap_entry_init(&k, strhash(normalized_key)); + hashmap_entry_init(&k.ent, strhash(normalized_key)); k.key = normalized_key; found_entry = hashmap_get(&cs->config_hash, &k, NULL); free(normalized_key); @@ -1882,7 +1882,7 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha */ if (!e) { e = xmalloc(sizeof(*e)); - hashmap_entry_init(e, strhash(key)); + hashmap_entry_init(&e->ent, strhash(key)); e->key = xstrdup(key); string_list_init(&e->value_list, 1); hashmap_add(&cs->config_hash, e); diff --git a/diffcore-rename.c b/diffcore-rename.c index 9624864858..44a3ab1e31 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -329,7 +329,7 @@ static void insert_file_table(struct repository *r, entry->index = index; entry->filespec = filespec; - hashmap_entry_init(entry, hash_filespec(r, filespec)); + hashmap_entry_init(&entry->entry, hash_filespec(r, filespec)); hashmap_add(table, entry); } diff --git a/hashmap.c b/hashmap.c index d42f01ff5a..6818c65174 100644 --- a/hashmap.c +++ b/hashmap.c @@ -293,13 +293,13 @@ const void *memintern(const void *data, size_t len) hashmap_init(&map, (hashmap_cmp_fn) pool_entry_cmp, NULL, 0); /* lookup interned string in pool */ - hashmap_entry_init(&key, memhash(data, len)); + hashmap_entry_init(&key.ent, memhash(data, len)); key.len = len; e = hashmap_get(&map, &key, data); if (!e) { /* not found: create it */ FLEX_ALLOC_MEM(e, data, data, len); - hashmap_entry_init(e, key.ent.hash); + hashmap_entry_init(&e->ent, key.ent.hash); e->len = len; hashmap_add(&map, e); } diff --git a/hashmap.h b/hashmap.h index 8424911566..54b0b8c698 100644 --- a/hashmap.h +++ b/hashmap.h @@ -48,14 +48,14 @@ * if (!strcmp("add", action)) { * struct long2string *e; * FLEX_ALLOC_STR(e, value, value); - * hashmap_entry_init(e, memhash(&key, sizeof(long))); + * hashmap_entry_init(&e->ent, memhash(&key, sizeof(long))); * e->key = key; * hashmap_add(&map, e); * } * * if (!strcmp("print_all_by_key", action)) { * struct long2string k, *e; - * hashmap_entry_init(&k, memhash(&key, sizeof(long))); + * hashmap_entry_init(&k->ent, memhash(&key, sizeof(long))); * k.key = key; * * flags &= ~COMPARE_VALUE; @@ -70,7 +70,7 @@ * if (!strcmp("has_exact_match", action)) { * struct long2string *e; * FLEX_ALLOC_STR(e, value, value); - * hashmap_entry_init(e, memhash(&key, sizeof(long))); + * hashmap_entry_init(&e->ent, memhash(&key, sizeof(long))); * e->key = key; * * flags |= COMPARE_VALUE; @@ -80,7 +80,7 @@ * * if (!strcmp("has_exact_match_no_heap_alloc", action)) { * struct long2string k; - * hashmap_entry_init(&k, memhash(&key, sizeof(long))); + * hashmap_entry_init(&k->ent, memhash(&key, sizeof(long))); * k.key = key; * * flags |= COMPARE_VALUE; @@ -244,9 +244,9 @@ void hashmap_free(struct hashmap *map, int free_entries); * your structure was allocated with xmalloc(), you can just free(3) it, * and if it is on stack, you can just let it go out of scope). */ -static inline void hashmap_entry_init(void *entry, unsigned int hash) +static inline void hashmap_entry_init(struct hashmap_entry *e, + unsigned int hash) { - struct hashmap_entry *e = entry; e->hash = hash; e->next = NULL; } diff --git a/merge-recursive.c b/merge-recursive.c index 6b812d67e3..6bc4f14ff4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -61,7 +61,7 @@ static struct dir_rename_entry *dir_rename_find_entry(struct hashmap *hashmap, if (dir == NULL) return NULL; - hashmap_entry_init(&key, strhash(dir)); + hashmap_entry_init(&key.ent, strhash(dir)); key.dir = dir; return hashmap_get(hashmap, &key, NULL); } @@ -85,7 +85,7 @@ static void dir_rename_init(struct hashmap *map) static void dir_rename_entry_init(struct dir_rename_entry *entry, char *directory) { - hashmap_entry_init(entry, strhash(directory)); + hashmap_entry_init(&entry->ent, strhash(directory)); entry->dir = directory; entry->non_unique_new_dir = 0; strbuf_init(&entry->new_dir, 0); @@ -97,7 +97,7 @@ static struct collision_entry *collision_find_entry(struct hashmap *hashmap, { struct collision_entry key; - hashmap_entry_init(&key, strhash(target_file)); + hashmap_entry_init(&key.ent, strhash(target_file)); key.target_file = target_file; return hashmap_get(hashmap, &key, NULL); } @@ -454,7 +454,7 @@ static int save_files_dirs(const struct object_id *oid, strbuf_addstr(base, path); FLEX_ALLOC_MEM(entry, path, base->buf, base->len); - hashmap_entry_init(entry, path_hash(entry->path)); + hashmap_entry_init(&entry->e, path_hash(entry->path)); hashmap_add(&opt->current_file_dir_set, entry); strbuf_setlen(base, baselen); @@ -731,7 +731,7 @@ static char *unique_path(struct merge_options *opt, const char *path, const char } FLEX_ALLOC_MEM(entry, path, newpath.buf, newpath.len); - hashmap_entry_init(entry, path_hash(entry->path)); + hashmap_entry_init(&entry->e, path_hash(entry->path)); hashmap_add(&opt->current_file_dir_set, entry); return strbuf_detach(&newpath, NULL); } @@ -2358,7 +2358,8 @@ static void compute_collisions(struct hashmap *collisions, if (!collision_ent) { collision_ent = xcalloc(1, sizeof(struct collision_entry)); - hashmap_entry_init(collision_ent, strhash(new_path)); + hashmap_entry_init(&collision_ent->ent, + strhash(new_path)); hashmap_put(collisions, collision_ent); collision_ent->target_file = new_path; } else { diff --git a/name-hash.c b/name-hash.c index 695908609f..1ce1417f7e 100644 --- a/name-hash.c +++ b/name-hash.c @@ -33,7 +33,7 @@ static struct dir_entry *find_dir_entry__hash(struct index_state *istate, const char *name, unsigned int namelen, unsigned int hash) { struct dir_entry key; - hashmap_entry_init(&key, hash); + hashmap_entry_init(&key.ent, hash); key.namelen = namelen; return hashmap_get(&istate->dir_hash, &key, name); } @@ -68,7 +68,7 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate, if (!dir) { /* not found, create it and add to hash table */ FLEX_ALLOC_MEM(dir, name, ce->name, namelen); - hashmap_entry_init(dir, memihash(ce->name, namelen)); + hashmap_entry_init(&dir->ent, memihash(ce->name, namelen)); dir->namelen = namelen; hashmap_add(&istate->dir_hash, dir); @@ -106,7 +106,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce) if (ce->ce_flags & CE_HASHED) return; ce->ce_flags |= CE_HASHED; - hashmap_entry_init(ce, memihash(ce->name, ce_namelen(ce))); + hashmap_entry_init(&ce->ent, memihash(ce->name, ce_namelen(ce))); hashmap_add(&istate->name_hash, ce); if (ignore_case) @@ -280,7 +280,7 @@ static struct dir_entry *hash_dir_entry_with_parent_and_prefix( dir = find_dir_entry__hash(istate, prefix->buf, prefix->len, hash); if (!dir) { FLEX_ALLOC_MEM(dir, name, prefix->buf, prefix->len); - hashmap_entry_init(dir, hash); + hashmap_entry_init(&dir->ent, hash); dir->namelen = prefix->len; dir->parent = parent; hashmap_add(&istate->dir_hash, dir); @@ -472,7 +472,7 @@ static void *lazy_name_thread_proc(void *_data) for (k = 0; k < d->istate->cache_nr; k++) { struct cache_entry *ce_k = d->istate->cache[k]; ce_k->ce_flags |= CE_HASHED; - hashmap_entry_init(ce_k, d->lazy_entries[k].hash_name); + hashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name); hashmap_add(&d->istate->name_hash, ce_k); } diff --git a/packfile.c b/packfile.c index 37fe0b73a6..96535eb86b 100644 --- a/packfile.c +++ b/packfile.c @@ -1487,7 +1487,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset, if (!delta_base_cache.cmpfn) hashmap_init(&delta_base_cache, delta_base_cache_hash_cmp, NULL, 0); - hashmap_entry_init(ent, pack_entry_hash(p, base_offset)); + hashmap_entry_init(&ent->ent, pack_entry_hash(p, base_offset)); hashmap_add(&delta_base_cache, ent); } diff --git a/patch-ids.c b/patch-ids.c index e8c150d0c9..a2da711678 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -83,7 +83,7 @@ static int init_patch_id_entry(struct patch_id *patch, if (commit_patch_id(commit, &ids->diffopts, &header_only_patch_id, 1, 0)) return -1; - hashmap_entry_init(patch, oidhash(&header_only_patch_id)); + hashmap_entry_init(&patch->ent, oidhash(&header_only_patch_id)); return 0; } diff --git a/range-diff.c b/range-diff.c index ba1e9a4265..32b29f9594 100644 --- a/range-diff.c +++ b/range-diff.c @@ -217,7 +217,7 @@ static void find_exact_matches(struct string_list *a, struct string_list *b) util->i = i; util->patch = a->items[i].string; util->diff = util->patch + util->diff_offset; - hashmap_entry_init(util, strhash(util->diff)); + hashmap_entry_init(&util->e, strhash(util->diff)); hashmap_add(&map, util); } @@ -228,7 +228,7 @@ static void find_exact_matches(struct string_list *a, struct string_list *b) util->i = i; util->patch = b->items[i].string; util->diff = util->patch + util->diff_offset; - hashmap_entry_init(util, strhash(util->diff)); + hashmap_entry_init(&util->e, strhash(util->diff)); other = hashmap_remove(&map, util, NULL); if (other) { if (other->matching >= 0) diff --git a/ref-filter.c b/ref-filter.c index f27cfc8c3e..206014c93d 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1565,7 +1565,8 @@ static void populate_worktree_map(struct hashmap *map, struct worktree **worktre struct ref_to_worktree_entry *entry; entry = xmalloc(sizeof(*entry)); entry->wt = worktrees[i]; - hashmap_entry_init(entry, strhash(worktrees[i]->head_ref)); + hashmap_entry_init(&entry->ent, + strhash(worktrees[i]->head_ref)); hashmap_add(map, entry); } diff --git a/refs.c b/refs.c index cd297ee4bd..c43ec59c6e 100644 --- a/refs.c +++ b/refs.c @@ -1796,7 +1796,7 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry( struct ref_store_hash_entry *entry; FLEX_ALLOC_STR(entry, name, name); - hashmap_entry_init(entry, strhash(name)); + hashmap_entry_init(&entry->ent, strhash(name)); entry->refs = refs; return entry; } diff --git a/remote.c b/remote.c index e50f7602ed..bd81cb71bc 100644 --- a/remote.c +++ b/remote.c @@ -158,7 +158,7 @@ static struct remote *make_remote(const char *name, int len) ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc); remotes[remotes_nr++] = ret; - hashmap_entry_init(ret, lookup_entry.hash); + hashmap_entry_init(&ret->ent, lookup_entry.hash); replaced = hashmap_put(&remotes_hash, ret); assert(replaced == NULL); /* no previous entry overwritten */ return ret; diff --git a/revision.c b/revision.c index 07412297f0..3461c78883 100644 --- a/revision.c +++ b/revision.c @@ -141,7 +141,7 @@ static void paths_and_oids_insert(struct hashmap *map, struct path_and_oids_entry key; struct path_and_oids_entry *entry; - hashmap_entry_init(&key, hash); + hashmap_entry_init(&key.ent, hash); /* use a shallow copy for the lookup */ key.path = (char *)path; @@ -149,7 +149,7 @@ static void paths_and_oids_insert(struct hashmap *map, if (!(entry = (struct path_and_oids_entry *)hashmap_get(map, &key, NULL))) { entry = xcalloc(1, sizeof(struct path_and_oids_entry)); - hashmap_entry_init(entry, hash); + hashmap_entry_init(&entry->ent, hash); entry->path = xstrdup(key.path); oidset_init(&entry->trees, 16); hashmap_put(map, entry); diff --git a/sequencer.c b/sequencer.c index 34ebf8ed94..1140cdf526 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4538,7 +4538,7 @@ static const char *label_oid(struct object_id *oid, const char *label, } FLEX_ALLOC_STR(labels_entry, label, label); - hashmap_entry_init(labels_entry, strihash(label)); + hashmap_entry_init(&labels_entry->entry, strihash(label)); hashmap_add(&state->labels, labels_entry); FLEX_ALLOC_STR(string_entry, string, label); @@ -5252,7 +5252,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list) strhash(subject), subject)) { FLEX_ALLOC_MEM(entry, subject, subject, subject_len); entry->i = i; - hashmap_entry_init(entry, strhash(entry->subject)); + hashmap_entry_init(&entry->entry, + strhash(entry->subject)); hashmap_put(&subject2item, entry); } } diff --git a/sub-process.c b/sub-process.c index 3f4af93555..9847dad6fc 100644 --- a/sub-process.c +++ b/sub-process.c @@ -20,7 +20,7 @@ struct subprocess_entry *subprocess_find_entry(struct hashmap *hashmap, const ch { struct subprocess_entry key; - hashmap_entry_init(&key, strhash(cmd)); + hashmap_entry_init(&key.ent, strhash(cmd)); key.cmd = cmd; return hashmap_get(hashmap, &key, NULL); } @@ -96,7 +96,7 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co return err; } - hashmap_entry_init(entry, strhash(cmd)); + hashmap_entry_init(&entry->ent, strhash(cmd)); err = startfn(entry); if (err) { diff --git a/submodule-config.c b/submodule-config.c index 4264ee216f..4aa02e280e 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -123,7 +123,7 @@ static void cache_put_path(struct submodule_cache *cache, unsigned int hash = hash_oid_string(&submodule->gitmodules_oid, submodule->path); struct submodule_entry *e = xmalloc(sizeof(*e)); - hashmap_entry_init(e, hash); + hashmap_entry_init(&e->ent, hash); e->config = submodule; hashmap_put(&cache->for_path, e); } @@ -135,7 +135,7 @@ static void cache_remove_path(struct submodule_cache *cache, submodule->path); struct submodule_entry e; struct submodule_entry *removed; - hashmap_entry_init(&e, hash); + hashmap_entry_init(&e.ent, hash); e.config = submodule; removed = hashmap_remove(&cache->for_path, &e, NULL); free(removed); @@ -147,7 +147,7 @@ static void cache_add(struct submodule_cache *cache, unsigned int hash = hash_oid_string(&submodule->gitmodules_oid, submodule->name); struct submodule_entry *e = xmalloc(sizeof(*e)); - hashmap_entry_init(e, hash); + hashmap_entry_init(&e->ent, hash); e->config = submodule; hashmap_add(&cache->for_name, e); } @@ -163,7 +163,7 @@ static const struct submodule *cache_lookup_path(struct submodule_cache *cache, oidcpy(&key_config.gitmodules_oid, gitmodules_oid); key_config.path = path; - hashmap_entry_init(&key, hash); + hashmap_entry_init(&key.ent, hash); key.config = &key_config; entry = hashmap_get(&cache->for_path, &key, NULL); @@ -183,7 +183,7 @@ static struct submodule *cache_lookup_name(struct submodule_cache *cache, oidcpy(&key_config.gitmodules_oid, gitmodules_oid); key_config.name = name; - hashmap_entry_init(&key, hash); + hashmap_entry_init(&key.ent, hash); key.config = &key_config; entry = hashmap_get(&cache->for_name, &key, NULL); diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index aaf17b0ddf..0c9fd7c996 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -37,7 +37,7 @@ static struct test_entry *alloc_test_entry(unsigned int hash, size_t klen = strlen(key); size_t vlen = strlen(value); struct test_entry *entry = xmalloc(st_add4(sizeof(*entry), klen, vlen, 2)); - hashmap_entry_init(entry, hash); + hashmap_entry_init(&entry->ent, hash); memcpy(entry->key, key, klen + 1); memcpy(entry->key + klen + 1, value, vlen + 1); return entry; @@ -103,7 +103,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds) /* add entries */ for (i = 0; i < TEST_SIZE; i++) { - hashmap_entry_init(entries[i], hashes[i]); + hashmap_entry_init(&entries[i]->ent, hashes[i]); hashmap_add(&map, entries[i]); } @@ -116,7 +116,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds) /* fill the map (sparsely if specified) */ j = (method & TEST_SPARSE) ? TEST_SIZE / 10 : TEST_SIZE; for (i = 0; i < j; i++) { - hashmap_entry_init(entries[i], hashes[i]); + hashmap_entry_init(&entries[i]->ent, hashes[i]); hashmap_add(&map, entries[i]); } From f6eb6bdcf2719defc3d38e0e2712fa3e18d29e91 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:28 +0000 Subject: [PATCH 05/20] hashmap_get_next takes "const struct hashmap_entry *" This is less error-prone than "const void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- diff.c | 5 +++-- diffcore-rename.c | 2 +- hashmap.c | 5 +++-- hashmap.h | 3 ++- name-hash.c | 2 +- t/helper/test-hashmap.c | 2 +- 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/diff.c b/diff.c index 02491ee684..1168f0cbb9 100644 --- a/diff.c +++ b/diff.c @@ -1036,7 +1036,7 @@ static void pmb_advance_or_null_multi_match(struct diff_options *o, int i; char *got_match = xcalloc(1, pmb_nr); - for (; match; match = hashmap_get_next(hm, match)) { + for (; match; match = hashmap_get_next(hm, &match->ent)) { for (i = 0; i < pmb_nr; i++) { struct moved_entry *prev = pmb[i].match; struct moved_entry *cur = (prev && prev->next_line) ? @@ -1189,7 +1189,8 @@ static void mark_color_as_moved(struct diff_options *o, * The current line is the start of a new block. * Setup the set of potential blocks. */ - for (; match; match = hashmap_get_next(hm, match)) { + for (; match; match = hashmap_get_next(hm, + &match->ent)) { ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc); if (o->color_moved_ws_handling & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) { diff --git a/diffcore-rename.c b/diffcore-rename.c index 44a3ab1e31..2a1449013b 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -285,7 +285,7 @@ static int find_identical_files(struct hashmap *srcs, p = hashmap_get_from_hash(srcs, hash_filespec(options->repo, target), NULL); - for (; p; p = hashmap_get_next(srcs, p)) { + for (; p; p = hashmap_get_next(srcs, &p->entry)) { int score; struct diff_filespec *source = p->filespec; diff --git a/hashmap.c b/hashmap.c index 6818c65174..c1de40eea0 100644 --- a/hashmap.c +++ b/hashmap.c @@ -191,9 +191,10 @@ void *hashmap_get(const struct hashmap *map, const void *key, const void *keydat return *find_entry_ptr(map, key, keydata); } -void *hashmap_get_next(const struct hashmap *map, const void *entry) +void *hashmap_get_next(const struct hashmap *map, + const struct hashmap_entry *entry) { - struct hashmap_entry *e = ((struct hashmap_entry *) entry)->next; + struct hashmap_entry *e = entry->next; for (; e; e = e->next) if (entry_equals(map, entry, e, NULL)) return e; diff --git a/hashmap.h b/hashmap.h index 54b0b8c698..93fb9599ca 100644 --- a/hashmap.h +++ b/hashmap.h @@ -318,7 +318,8 @@ static inline void *hashmap_get_from_hash(const struct hashmap *map, * `entry` is the hashmap_entry to start the search from, obtained via a previous * call to `hashmap_get` or `hashmap_get_next`. */ -void *hashmap_get_next(const struct hashmap *map, const void *entry); +void *hashmap_get_next(const struct hashmap *map, + const struct hashmap_entry *entry); /* * Adds a hashmap entry. This allows to add duplicate entries (i.e. diff --git a/name-hash.c b/name-hash.c index 1ce1417f7e..4d84326c58 100644 --- a/name-hash.c +++ b/name-hash.c @@ -710,7 +710,7 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na while (ce) { if (same_name(ce, name, namelen, icase)) return ce; - ce = hashmap_get_next(&istate->name_hash, ce); + ce = hashmap_get_next(&istate->name_hash, &ce->ent); } return NULL; } diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index 0c9fd7c996..bf063a2521 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -203,7 +203,7 @@ int cmd__hashmap(int argc, const char **argv) puts("NULL"); while (entry) { puts(get_value(entry)); - entry = hashmap_get_next(&map, entry); + entry = hashmap_get_next(&map, &entry->ent); } } else if (!strcmp("remove", cmd) && p1) { From b94e5c1df674eb4ec8fdeaaae1ad8df552cb5d70 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:29 +0000 Subject: [PATCH 06/20] hashmap_add takes "struct hashmap_entry *" This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 2 +- blame.c | 2 +- builtin/describe.c | 2 +- builtin/difftool.c | 6 +++--- builtin/fetch.c | 2 +- config.c | 2 +- diff.c | 2 +- diffcore-rename.c | 2 +- hashmap.c | 6 +++--- hashmap.h | 4 ++-- merge-recursive.c | 4 ++-- name-hash.c | 8 ++++---- packfile.c | 2 +- patch-ids.c | 2 +- range-diff.c | 2 +- ref-filter.c | 2 +- sequencer.c | 2 +- sub-process.c | 2 +- submodule-config.c | 2 +- t/helper/test-hashmap.c | 6 +++--- 20 files changed, 31 insertions(+), 31 deletions(-) diff --git a/attr.c b/attr.c index a8be7a7b4f..fa26a3e3cb 100644 --- a/attr.c +++ b/attr.c @@ -122,7 +122,7 @@ static void attr_hashmap_add(struct attr_hashmap *map, e->keylen = keylen; e->value = value; - hashmap_add(&map->map, e); + hashmap_add(&map->map, &e->ent); } struct all_attrs_item { diff --git a/blame.c b/blame.c index 46059410cd..4d20aee435 100644 --- a/blame.c +++ b/blame.c @@ -424,7 +424,7 @@ static void get_fingerprint(struct fingerprint *result, found_entry->count += 1; } else { entry->count = 1; - hashmap_add(&result->map, entry); + hashmap_add(&result->map, &entry->entry); ++entry; } } diff --git a/builtin/describe.c b/builtin/describe.c index 596ddf89a5..f5e0a7e033 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -124,7 +124,7 @@ static void add_to_known_names(const char *path, e = xmalloc(sizeof(struct commit_name)); oidcpy(&e->peeled, peeled); hashmap_entry_init(&e->entry, oidhash(peeled)); - hashmap_add(&names, e); + hashmap_add(&names, &e->entry); e->path = NULL; } e->tag = tag; diff --git a/builtin/difftool.c b/builtin/difftool.c index 98ffc04c61..82c146718d 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -168,7 +168,7 @@ static void add_left_or_right(struct hashmap *map, const char *path, e = existing; } else { e->left[0] = e->right[0] = '\0'; - hashmap_add(map, e); + hashmap_add(map, &e->entry); } strlcpy(is_right ? e->right : e->left, content, PATH_MAX); } @@ -235,7 +235,7 @@ static void changed_files(struct hashmap *result, const char *index_path, struct path_entry *entry; FLEX_ALLOC_STR(entry, path, buf.buf); hashmap_entry_init(&entry->entry, strhash(buf.buf)); - hashmap_add(result, entry); + hashmap_add(result, &entry->entry); } fclose(fp); if (finish_command(&diff_files)) @@ -466,7 +466,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, free(entry); continue; } - hashmap_add(&working_tree_dups, entry); + hashmap_add(&working_tree_dups, &entry->entry); if (!use_wt_file(workdir, dst_path, &roid)) { if (checkout_path(rmode, &roid, dst_path, diff --git a/builtin/fetch.c b/builtin/fetch.c index b7d70eee70..909dbde909 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -278,7 +278,7 @@ static struct refname_hash_entry *refname_hash_add(struct hashmap *map, FLEX_ALLOC_MEM(ent, refname, refname, len); hashmap_entry_init(&ent->ent, strhash(refname)); oidcpy(&ent->oid, oid); - hashmap_add(map, ent); + hashmap_add(map, &ent->ent); return ent; } diff --git a/config.c b/config.c index 08d866e7de..2243d7c3d6 100644 --- a/config.c +++ b/config.c @@ -1885,7 +1885,7 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha hashmap_entry_init(&e->ent, strhash(key)); e->key = xstrdup(key); string_list_init(&e->value_list, 1); - hashmap_add(&cs->config_hash, e); + hashmap_add(&cs->config_hash, &e->ent); } si = string_list_append_nodup(&e->value_list, xstrdup_or_null(value)); diff --git a/diff.c b/diff.c index 1168f0cbb9..cc7f06d10d 100644 --- a/diff.c +++ b/diff.c @@ -1003,7 +1003,7 @@ static void add_lines_to_move_detection(struct diff_options *o, if (prev_line && prev_line->es->s == o->emitted_symbols->buf[n].s) prev_line->next_line = key; - hashmap_add(hm, key); + hashmap_add(hm, &key->ent); prev_line = key; } } diff --git a/diffcore-rename.c b/diffcore-rename.c index 2a1449013b..4670a40179 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -330,7 +330,7 @@ static void insert_file_table(struct repository *r, entry->filespec = filespec; hashmap_entry_init(&entry->entry, hash_filespec(r, filespec)); - hashmap_add(table, entry); + hashmap_add(table, &entry->entry); } /* diff --git a/hashmap.c b/hashmap.c index c1de40eea0..9c2db3e0c8 100644 --- a/hashmap.c +++ b/hashmap.c @@ -201,12 +201,12 @@ void *hashmap_get_next(const struct hashmap *map, return NULL; } -void hashmap_add(struct hashmap *map, void *entry) +void hashmap_add(struct hashmap *map, struct hashmap_entry *entry) { unsigned int b = bucket(map, entry); /* add entry */ - ((struct hashmap_entry *) entry)->next = map->table[b]; + entry->next = map->table[b]; map->table[b] = entry; /* fix size and rehash if appropriate */ @@ -302,7 +302,7 @@ const void *memintern(const void *data, size_t len) FLEX_ALLOC_MEM(e, data, data, len); hashmap_entry_init(&e->ent, key.ent.hash); e->len = len; - hashmap_add(&map, e); + hashmap_add(&map, &e->ent); } return e->data; } diff --git a/hashmap.h b/hashmap.h index 93fb9599ca..47ee5c00d7 100644 --- a/hashmap.h +++ b/hashmap.h @@ -50,7 +50,7 @@ * FLEX_ALLOC_STR(e, value, value); * hashmap_entry_init(&e->ent, memhash(&key, sizeof(long))); * e->key = key; - * hashmap_add(&map, e); + * hashmap_add(&map, &e->ent); * } * * if (!strcmp("print_all_by_key", action)) { @@ -328,7 +328,7 @@ void *hashmap_get_next(const struct hashmap *map, * `map` is the hashmap structure. * `entry` is the entry to add. */ -void hashmap_add(struct hashmap *map, void *entry); +void hashmap_add(struct hashmap *map, struct hashmap_entry *entry); /* * Adds or replaces a hashmap entry. If the hashmap contains duplicate diff --git a/merge-recursive.c b/merge-recursive.c index 6bc4f14ff4..db9b247ece 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -455,7 +455,7 @@ static int save_files_dirs(const struct object_id *oid, FLEX_ALLOC_MEM(entry, path, base->buf, base->len); hashmap_entry_init(&entry->e, path_hash(entry->path)); - hashmap_add(&opt->current_file_dir_set, entry); + hashmap_add(&opt->current_file_dir_set, &entry->e); strbuf_setlen(base, baselen); return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0); @@ -732,7 +732,7 @@ static char *unique_path(struct merge_options *opt, const char *path, const char FLEX_ALLOC_MEM(entry, path, newpath.buf, newpath.len); hashmap_entry_init(&entry->e, path_hash(entry->path)); - hashmap_add(&opt->current_file_dir_set, entry); + hashmap_add(&opt->current_file_dir_set, &entry->e); return strbuf_detach(&newpath, NULL); } diff --git a/name-hash.c b/name-hash.c index 4d84326c58..faec682bc7 100644 --- a/name-hash.c +++ b/name-hash.c @@ -70,7 +70,7 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate, FLEX_ALLOC_MEM(dir, name, ce->name, namelen); hashmap_entry_init(&dir->ent, memihash(ce->name, namelen)); dir->namelen = namelen; - hashmap_add(&istate->dir_hash, dir); + hashmap_add(&istate->dir_hash, &dir->ent); /* recursively add missing parent directories */ dir->parent = hash_dir_entry(istate, ce, namelen); @@ -107,7 +107,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce) return; ce->ce_flags |= CE_HASHED; hashmap_entry_init(&ce->ent, memihash(ce->name, ce_namelen(ce))); - hashmap_add(&istate->name_hash, ce); + hashmap_add(&istate->name_hash, &ce->ent); if (ignore_case) add_dir_entry(istate, ce); @@ -283,7 +283,7 @@ static struct dir_entry *hash_dir_entry_with_parent_and_prefix( hashmap_entry_init(&dir->ent, hash); dir->namelen = prefix->len; dir->parent = parent; - hashmap_add(&istate->dir_hash, dir); + hashmap_add(&istate->dir_hash, &dir->ent); if (parent) { unlock_dir_mutex(lock_nr); @@ -473,7 +473,7 @@ static void *lazy_name_thread_proc(void *_data) struct cache_entry *ce_k = d->istate->cache[k]; ce_k->ce_flags |= CE_HASHED; hashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name); - hashmap_add(&d->istate->name_hash, ce_k); + hashmap_add(&d->istate->name_hash, &ce_k->ent); } return NULL; diff --git a/packfile.c b/packfile.c index 96535eb86b..f7402c470b 100644 --- a/packfile.c +++ b/packfile.c @@ -1488,7 +1488,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset, if (!delta_base_cache.cmpfn) hashmap_init(&delta_base_cache, delta_base_cache_hash_cmp, NULL, 0); hashmap_entry_init(&ent->ent, pack_entry_hash(p, base_offset)); - hashmap_add(&delta_base_cache, ent); + hashmap_add(&delta_base_cache, &ent->ent); } int packed_object_info(struct repository *r, struct packed_git *p, diff --git a/patch-ids.c b/patch-ids.c index a2da711678..f87b62bf58 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -116,6 +116,6 @@ struct patch_id *add_commit_patch_id(struct commit *commit, return NULL; } - hashmap_add(&ids->patches, key); + hashmap_add(&ids->patches, &key->ent); return key; } diff --git a/range-diff.c b/range-diff.c index 32b29f9594..96f955d84d 100644 --- a/range-diff.c +++ b/range-diff.c @@ -218,7 +218,7 @@ static void find_exact_matches(struct string_list *a, struct string_list *b) util->patch = a->items[i].string; util->diff = util->patch + util->diff_offset; hashmap_entry_init(&util->e, strhash(util->diff)); - hashmap_add(&map, util); + hashmap_add(&map, &util->e); } /* Now try to find exact matches in b */ diff --git a/ref-filter.c b/ref-filter.c index 206014c93d..d939ebc6bb 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1568,7 +1568,7 @@ static void populate_worktree_map(struct hashmap *map, struct worktree **worktre hashmap_entry_init(&entry->ent, strhash(worktrees[i]->head_ref)); - hashmap_add(map, entry); + hashmap_add(map, &entry->ent); } } } diff --git a/sequencer.c b/sequencer.c index 1140cdf526..ee11cda7e7 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4539,7 +4539,7 @@ static const char *label_oid(struct object_id *oid, const char *label, FLEX_ALLOC_STR(labels_entry, label, label); hashmap_entry_init(&labels_entry->entry, strihash(label)); - hashmap_add(&state->labels, labels_entry); + hashmap_add(&state->labels, &labels_entry->entry); FLEX_ALLOC_STR(string_entry, string, label); oidcpy(&string_entry->entry.oid, oid); diff --git a/sub-process.c b/sub-process.c index 9847dad6fc..d58e069855 100644 --- a/sub-process.c +++ b/sub-process.c @@ -105,7 +105,7 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co return err; } - hashmap_add(hashmap, entry); + hashmap_add(hashmap, &entry->ent); return 0; } diff --git a/submodule-config.c b/submodule-config.c index 4aa02e280e..a3bbd9fd6f 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -149,7 +149,7 @@ static void cache_add(struct submodule_cache *cache, struct submodule_entry *e = xmalloc(sizeof(*e)); hashmap_entry_init(&e->ent, hash); e->config = submodule; - hashmap_add(&cache->for_name, e); + hashmap_add(&cache->for_name, &e->ent); } static const struct submodule *cache_lookup_path(struct submodule_cache *cache, diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index bf063a2521..49e715f1cd 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -104,7 +104,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds) /* add entries */ for (i = 0; i < TEST_SIZE; i++) { hashmap_entry_init(&entries[i]->ent, hashes[i]); - hashmap_add(&map, entries[i]); + hashmap_add(&map, &entries[i]->ent); } hashmap_free(&map, 0); @@ -117,7 +117,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds) j = (method & TEST_SPARSE) ? TEST_SIZE / 10 : TEST_SIZE; for (i = 0; i < j; i++) { hashmap_entry_init(&entries[i]->ent, hashes[i]); - hashmap_add(&map, entries[i]); + hashmap_add(&map, &entries[i]->ent); } for (j = 0; j < rounds; j++) { @@ -179,7 +179,7 @@ int cmd__hashmap(int argc, const char **argv) entry = alloc_test_entry(hash, p1, p2); /* add to hashmap */ - hashmap_add(&map, entry); + hashmap_add(&map, &entry->ent); } else if (!strcmp("put", cmd) && p1 && p2) { From b6c5241606e67b57470e86ccf547d4ab90008a1d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:30 +0000 Subject: [PATCH 07/20] hashmap_get takes "const struct hashmap_entry *" This is less error-prone than "const void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 2 +- blame.c | 6 +++--- builtin/difftool.c | 5 +++-- builtin/fast-export.c | 2 +- config.c | 2 +- diff.c | 4 ++-- hashmap.c | 5 +++-- hashmap.h | 8 +++++--- merge-recursive.c | 4 ++-- name-hash.c | 2 +- patch-ids.c | 2 +- revision.c | 3 ++- sub-process.c | 2 +- submodule-config.c | 4 ++-- 14 files changed, 28 insertions(+), 23 deletions(-) diff --git a/attr.c b/attr.c index fa26a3e3cb..9bdef61cc3 100644 --- a/attr.c +++ b/attr.c @@ -101,7 +101,7 @@ static void *attr_hashmap_get(struct attr_hashmap *map, hashmap_entry_init(&k.ent, memhash(key, keylen)); k.key = key; k.keylen = keylen; - e = hashmap_get(&map->map, &k, NULL); + e = hashmap_get(&map->map, &k.ent, NULL); return e ? e->value : NULL; } diff --git a/blame.c b/blame.c index 4d20aee435..73ffb59403 100644 --- a/blame.c +++ b/blame.c @@ -419,7 +419,7 @@ static void get_fingerprint(struct fingerprint *result, continue; hashmap_entry_init(&entry->entry, hash); - found_entry = hashmap_get(&result->map, entry, NULL); + found_entry = hashmap_get(&result->map, &entry->entry, NULL); if (found_entry) { found_entry->count += 1; } else { @@ -452,7 +452,7 @@ static int fingerprint_similarity(struct fingerprint *a, struct fingerprint *b) hashmap_iter_init(&b->map, &iter); while ((entry_b = hashmap_iter_next(&iter))) { - if ((entry_a = hashmap_get(&a->map, entry_b, NULL))) { + if ((entry_a = hashmap_get(&a->map, &entry_b->entry, NULL))) { intersection += entry_a->count < entry_b->count ? entry_a->count : entry_b->count; } @@ -471,7 +471,7 @@ static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b) hashmap_iter_init(&b->map, &iter); while ((entry_b = hashmap_iter_next(&iter))) { - if ((entry_a = hashmap_get(&a->map, entry_b, NULL))) { + if ((entry_a = hashmap_get(&a->map, &entry_b->entry, NULL))) { if (entry_a->count <= entry_b->count) hashmap_remove(&a->map, entry_b, NULL); else diff --git a/builtin/difftool.c b/builtin/difftool.c index 82c146718d..f41298d199 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -162,7 +162,7 @@ static void add_left_or_right(struct hashmap *map, const char *path, FLEX_ALLOC_STR(e, path, path); hashmap_entry_init(&e->entry, strhash(path)); - existing = hashmap_get(map, e, NULL); + existing = hashmap_get(map, &e->entry, NULL); if (existing) { free(e); e = existing; @@ -462,7 +462,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, /* Avoid duplicate working_tree entries */ FLEX_ALLOC_STR(entry, path, dst_path); hashmap_entry_init(&entry->entry, strhash(dst_path)); - if (hashmap_get(&working_tree_dups, entry, NULL)) { + if (hashmap_get(&working_tree_dups, &entry->entry, + NULL)) { free(entry); continue; } diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 287dbd24a3..c693cf6a8c 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -151,7 +151,7 @@ static const void *anonymize_mem(struct hashmap *map, hashmap_entry_init(&key.hash, memhash(orig, *len)); key.orig = orig; key.orig_len = *len; - ret = hashmap_get(map, &key, NULL); + ret = hashmap_get(map, &key.hash, NULL); if (!ret) { ret = xmalloc(sizeof(*ret)); diff --git a/config.c b/config.c index 2243d7c3d6..1a1b6675fd 100644 --- a/config.c +++ b/config.c @@ -1863,7 +1863,7 @@ static struct config_set_element *configset_find_element(struct config_set *cs, hashmap_entry_init(&k.ent, strhash(normalized_key)); k.key = normalized_key; - found_entry = hashmap_get(&cs->config_hash, &k, NULL); + found_entry = hashmap_get(&cs->config_hash, &k.ent, NULL); free(normalized_key); return found_entry; } diff --git a/diff.c b/diff.c index cc7f06d10d..72d3c6aa19 100644 --- a/diff.c +++ b/diff.c @@ -1144,13 +1144,13 @@ static void mark_color_as_moved(struct diff_options *o, case DIFF_SYMBOL_PLUS: hm = del_lines; key = prepare_entry(o, n); - match = hashmap_get(hm, key, NULL); + match = hashmap_get(hm, &key->ent, NULL); free(key); break; case DIFF_SYMBOL_MINUS: hm = add_lines; key = prepare_entry(o, n); - match = hashmap_get(hm, key, NULL); + match = hashmap_get(hm, &key->ent, NULL); free(key); break; default: diff --git a/hashmap.c b/hashmap.c index 9c2db3e0c8..092236c09a 100644 --- a/hashmap.c +++ b/hashmap.c @@ -186,7 +186,8 @@ void hashmap_free(struct hashmap *map, int free_entries) memset(map, 0, sizeof(*map)); } -void *hashmap_get(const struct hashmap *map, const void *key, const void *keydata) +void *hashmap_get(const struct hashmap *map, const struct hashmap_entry *key, + const void *keydata) { return *find_entry_ptr(map, key, keydata); } @@ -296,7 +297,7 @@ const void *memintern(const void *data, size_t len) /* lookup interned string in pool */ hashmap_entry_init(&key.ent, memhash(data, len)); key.len = len; - e = hashmap_get(&map, &key, data); + e = hashmap_get(&map, &key.ent, data); if (!e) { /* not found: create it */ FLEX_ALLOC_MEM(e, data, data, len); diff --git a/hashmap.h b/hashmap.h index 47ee5c00d7..ef83f9431d 100644 --- a/hashmap.h +++ b/hashmap.h @@ -74,7 +74,8 @@ * e->key = key; * * flags |= COMPARE_VALUE; - * printf("%sfound\n", hashmap_get(&map, e, NULL) ? "" : "not "); + * printf("%sfound\n", + * hashmap_get(&map, &e->ent, NULL) ? "" : "not "); * free(e); * } * @@ -84,7 +85,8 @@ * k.key = key; * * flags |= COMPARE_VALUE; - * printf("%sfound\n", hashmap_get(&map, &k, value) ? "" : "not "); + * printf("%sfound\n", + * hashmap_get(&map, &k->ent, value) ? "" : "not "); * } * * if (!strcmp("end", action)) { @@ -286,7 +288,7 @@ static inline unsigned int hashmap_get_size(struct hashmap *map) * If an entry with matching hash code is found, `key` and `keydata` are passed * to `hashmap_cmp_fn` to decide whether the entry matches the key. */ -void *hashmap_get(const struct hashmap *map, const void *key, +void *hashmap_get(const struct hashmap *map, const struct hashmap_entry *key, const void *keydata); /* diff --git a/merge-recursive.c b/merge-recursive.c index db9b247ece..2d31a3e279 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -63,7 +63,7 @@ static struct dir_rename_entry *dir_rename_find_entry(struct hashmap *hashmap, return NULL; hashmap_entry_init(&key.ent, strhash(dir)); key.dir = dir; - return hashmap_get(hashmap, &key, NULL); + return hashmap_get(hashmap, &key.ent, NULL); } static int dir_rename_cmp(const void *unused_cmp_data, @@ -99,7 +99,7 @@ static struct collision_entry *collision_find_entry(struct hashmap *hashmap, hashmap_entry_init(&key.ent, strhash(target_file)); key.target_file = target_file; - return hashmap_get(hashmap, &key, NULL); + return hashmap_get(hashmap, &key.ent, NULL); } static int collision_cmp(void *unused_cmp_data, diff --git a/name-hash.c b/name-hash.c index faec682bc7..4eaeded775 100644 --- a/name-hash.c +++ b/name-hash.c @@ -35,7 +35,7 @@ static struct dir_entry *find_dir_entry__hash(struct index_state *istate, struct dir_entry key; hashmap_entry_init(&key.ent, hash); key.namelen = namelen; - return hashmap_get(&istate->dir_hash, &key, name); + return hashmap_get(&istate->dir_hash, &key.ent, name); } static struct dir_entry *find_dir_entry(struct index_state *istate, diff --git a/patch-ids.c b/patch-ids.c index f87b62bf58..437f29e42c 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -99,7 +99,7 @@ struct patch_id *has_commit_patch_id(struct commit *commit, if (init_patch_id_entry(&patch, commit, ids)) return NULL; - return hashmap_get(&ids->patches, &patch, NULL); + return hashmap_get(&ids->patches, &patch.ent, NULL); } struct patch_id *add_commit_patch_id(struct commit *commit, diff --git a/revision.c b/revision.c index 3461c78883..4336281286 100644 --- a/revision.c +++ b/revision.c @@ -147,7 +147,8 @@ static void paths_and_oids_insert(struct hashmap *map, key.path = (char *)path; oidset_init(&key.trees, 0); - if (!(entry = (struct path_and_oids_entry *)hashmap_get(map, &key, NULL))) { + entry = hashmap_get(map, &key.ent, NULL); + if (!entry) { entry = xcalloc(1, sizeof(struct path_and_oids_entry)); hashmap_entry_init(&entry->ent, hash); entry->path = xstrdup(key.path); diff --git a/sub-process.c b/sub-process.c index d58e069855..debd86bb68 100644 --- a/sub-process.c +++ b/sub-process.c @@ -22,7 +22,7 @@ struct subprocess_entry *subprocess_find_entry(struct hashmap *hashmap, const ch hashmap_entry_init(&key.ent, strhash(cmd)); key.cmd = cmd; - return hashmap_get(hashmap, &key, NULL); + return hashmap_get(hashmap, &key.ent, NULL); } int subprocess_read_status(int fd, struct strbuf *status) diff --git a/submodule-config.c b/submodule-config.c index a3bbd9fd6f..58d585cd7d 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -166,7 +166,7 @@ static const struct submodule *cache_lookup_path(struct submodule_cache *cache, hashmap_entry_init(&key.ent, hash); key.config = &key_config; - entry = hashmap_get(&cache->for_path, &key, NULL); + entry = hashmap_get(&cache->for_path, &key.ent, NULL); if (entry) return entry->config; return NULL; @@ -186,7 +186,7 @@ static struct submodule *cache_lookup_name(struct submodule_cache *cache, hashmap_entry_init(&key.ent, hash); key.config = &key_config; - entry = hashmap_get(&cache->for_name, &key, NULL); + entry = hashmap_get(&cache->for_name, &key.ent, NULL); if (entry) return entry->config; return NULL; From 28ee7941280828f9e528bd8c5d0f6515a57e0c44 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:31 +0000 Subject: [PATCH 08/20] hashmap_remove takes "const struct hashmap_entry *" This is less error-prone than "const void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- blame.c | 2 +- hashmap.c | 3 ++- hashmap.h | 2 +- merge-recursive.c | 2 +- name-hash.c | 4 ++-- packfile.c | 2 +- range-diff.c | 2 +- sub-process.c | 2 +- submodule-config.c | 2 +- 9 files changed, 11 insertions(+), 10 deletions(-) diff --git a/blame.c b/blame.c index 73ffb59403..00f8f3fb0a 100644 --- a/blame.c +++ b/blame.c @@ -473,7 +473,7 @@ static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b) while ((entry_b = hashmap_iter_next(&iter))) { if ((entry_a = hashmap_get(&a->map, &entry_b->entry, NULL))) { if (entry_a->count <= entry_b->count) - hashmap_remove(&a->map, entry_b, NULL); + hashmap_remove(&a->map, &entry_b->entry, NULL); else entry_a->count -= entry_b->count; } diff --git a/hashmap.c b/hashmap.c index 092236c09a..bdf33e0381 100644 --- a/hashmap.c +++ b/hashmap.c @@ -218,7 +218,8 @@ void hashmap_add(struct hashmap *map, struct hashmap_entry *entry) } } -void *hashmap_remove(struct hashmap *map, const void *key, const void *keydata) +void *hashmap_remove(struct hashmap *map, const struct hashmap_entry *key, + const void *keydata) { struct hashmap_entry *old; struct hashmap_entry **e = find_entry_ptr(map, key, keydata); diff --git a/hashmap.h b/hashmap.h index ef83f9431d..c4c31b462f 100644 --- a/hashmap.h +++ b/hashmap.h @@ -349,7 +349,7 @@ void *hashmap_put(struct hashmap *map, void *entry); * * Argument explanation is the same as in `hashmap_get`. */ -void *hashmap_remove(struct hashmap *map, const void *key, +void *hashmap_remove(struct hashmap *map, const struct hashmap_entry *key, const void *keydata); /* diff --git a/merge-recursive.c b/merge-recursive.c index 2d31a3e279..f60451d396 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2001,7 +2001,7 @@ static void remove_hashmap_entries(struct hashmap *dir_renames, for (i = 0; i < items_to_remove->nr; i++) { entry = items_to_remove->items[i].util; - hashmap_remove(dir_renames, entry, NULL); + hashmap_remove(dir_renames, &entry->ent, NULL); } string_list_clear(items_to_remove, 0); } diff --git a/name-hash.c b/name-hash.c index 4eaeded775..44d788f1ce 100644 --- a/name-hash.c +++ b/name-hash.c @@ -95,7 +95,7 @@ static void remove_dir_entry(struct index_state *istate, struct cache_entry *ce) struct dir_entry *dir = hash_dir_entry(istate, ce, ce_namelen(ce)); while (dir && !(--dir->nr)) { struct dir_entry *parent = dir->parent; - hashmap_remove(&istate->dir_hash, dir, NULL); + hashmap_remove(&istate->dir_hash, &dir->ent, NULL); free(dir); dir = parent; } @@ -625,7 +625,7 @@ void remove_name_hash(struct index_state *istate, struct cache_entry *ce) if (!istate->name_hash_initialized || !(ce->ce_flags & CE_HASHED)) return; ce->ce_flags &= ~CE_HASHED; - hashmap_remove(&istate->name_hash, ce, ce); + hashmap_remove(&istate->name_hash, &ce->ent, ce); if (ignore_case) remove_dir_entry(istate, ce); diff --git a/packfile.c b/packfile.c index f7402c470b..3edd648de0 100644 --- a/packfile.c +++ b/packfile.c @@ -1423,7 +1423,7 @@ static int in_delta_base_cache(struct packed_git *p, off_t base_offset) */ static void detach_delta_base_cache_entry(struct delta_base_cache_entry *ent) { - hashmap_remove(&delta_base_cache, ent, &ent->key); + hashmap_remove(&delta_base_cache, &ent->ent, &ent->key); list_del(&ent->lru); delta_base_cached -= ent->size; free(ent); diff --git a/range-diff.c b/range-diff.c index 96f955d84d..c51cfd5556 100644 --- a/range-diff.c +++ b/range-diff.c @@ -229,7 +229,7 @@ static void find_exact_matches(struct string_list *a, struct string_list *b) util->patch = b->items[i].string; util->diff = util->patch + util->diff_offset; hashmap_entry_init(&util->e, strhash(util->diff)); - other = hashmap_remove(&map, util, NULL); + other = hashmap_remove(&map, &util->e, NULL); if (other) { if (other->matching >= 0) BUG("already assigned!"); diff --git a/sub-process.c b/sub-process.c index debd86bb68..99fccef592 100644 --- a/sub-process.c +++ b/sub-process.c @@ -58,7 +58,7 @@ void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry) kill(entry->process.pid, SIGTERM); finish_command(&entry->process); - hashmap_remove(hashmap, entry, NULL); + hashmap_remove(hashmap, &entry->ent, NULL); } static void subprocess_exit_handler(struct child_process *process) diff --git a/submodule-config.c b/submodule-config.c index 58d585cd7d..7486745a6a 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -137,7 +137,7 @@ static void cache_remove_path(struct submodule_cache *cache, struct submodule_entry *removed; hashmap_entry_init(&e.ent, hash); e.config = submodule; - removed = hashmap_remove(&cache->for_path, &e, NULL); + removed = hashmap_remove(&cache->for_path, &e.ent, NULL); free(removed); } From 26b455f21ed7e0c7b0e4e4e69b5ae48545597020 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:32 +0000 Subject: [PATCH 09/20] hashmap_put takes "struct hashmap_entry *" This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/fast-export.c | 2 +- hashmap.c | 2 +- hashmap.h | 2 +- merge-recursive.c | 4 ++-- oidmap.c | 2 +- refs.c | 5 ++++- remote.c | 2 +- revision.c | 2 +- sequencer.c | 2 +- submodule-config.c | 2 +- t/helper/test-hashmap.c | 2 +- 11 files changed, 15 insertions(+), 12 deletions(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index c693cf6a8c..192e21dae4 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -160,7 +160,7 @@ static const void *anonymize_mem(struct hashmap *map, ret->orig_len = *len; ret->anon = generate(orig, len); ret->anon_len = *len; - hashmap_put(map, ret); + hashmap_put(map, &ret->hash); } *len = ret->anon_len; diff --git a/hashmap.c b/hashmap.c index bdf33e0381..9b83e73d03 100644 --- a/hashmap.c +++ b/hashmap.c @@ -241,7 +241,7 @@ void *hashmap_remove(struct hashmap *map, const struct hashmap_entry *key, return old; } -void *hashmap_put(struct hashmap *map, void *entry) +void *hashmap_put(struct hashmap *map, struct hashmap_entry *entry) { struct hashmap_entry *old = hashmap_remove(map, entry, NULL); hashmap_add(map, entry); diff --git a/hashmap.h b/hashmap.h index c4c31b462f..d122c75d0f 100644 --- a/hashmap.h +++ b/hashmap.h @@ -340,7 +340,7 @@ void hashmap_add(struct hashmap *map, struct hashmap_entry *entry); * `entry` is the entry to add or replace. * Returns the replaced entry, or NULL if not found (i.e. the entry was added). */ -void *hashmap_put(struct hashmap *map, void *entry); +void *hashmap_put(struct hashmap *map, struct hashmap_entry *entry); /* * Removes a hashmap entry matching the specified key. If the hashmap contains diff --git a/merge-recursive.c b/merge-recursive.c index f60451d396..a685b4fb69 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2229,7 +2229,7 @@ static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs) if (!entry) { entry = xmalloc(sizeof(*entry)); dir_rename_entry_init(entry, old_dir); - hashmap_put(dir_renames, entry); + hashmap_put(dir_renames, &entry->ent); } else { free(old_dir); } @@ -2360,7 +2360,7 @@ static void compute_collisions(struct hashmap *collisions, sizeof(struct collision_entry)); hashmap_entry_init(&collision_ent->ent, strhash(new_path)); - hashmap_put(collisions, collision_ent); + hashmap_put(collisions, &collision_ent->ent); collision_ent->target_file = new_path; } else { free(new_path); diff --git a/oidmap.c b/oidmap.c index 6d6e840d03..cd22b3a8bf 100644 --- a/oidmap.c +++ b/oidmap.c @@ -51,5 +51,5 @@ void *oidmap_put(struct oidmap *map, void *entry) oidmap_init(map, 0); hashmap_entry_init(&to_put->internal_entry, oidhash(&to_put->oid)); - return hashmap_put(&map->map, to_put); + return hashmap_put(&map->map, &to_put->internal_entry); } diff --git a/refs.c b/refs.c index c43ec59c6e..3e55031256 100644 --- a/refs.c +++ b/refs.c @@ -1863,10 +1863,13 @@ static void register_ref_store_map(struct hashmap *map, struct ref_store *refs, const char *name) { + struct ref_store_hash_entry *entry; + if (!map->tablesize) hashmap_init(map, ref_store_hash_cmp, NULL, 0); - if (hashmap_put(map, alloc_ref_store_hash_entry(name, refs))) + entry = alloc_ref_store_hash_entry(name, refs); + if (hashmap_put(map, &entry->ent)) BUG("%s ref_store '%s' initialized twice", type, name); } diff --git a/remote.c b/remote.c index bd81cb71bc..8ca23d95dc 100644 --- a/remote.c +++ b/remote.c @@ -159,7 +159,7 @@ static struct remote *make_remote(const char *name, int len) remotes[remotes_nr++] = ret; hashmap_entry_init(&ret->ent, lookup_entry.hash); - replaced = hashmap_put(&remotes_hash, ret); + replaced = hashmap_put(&remotes_hash, &ret->ent); assert(replaced == NULL); /* no previous entry overwritten */ return ret; } diff --git a/revision.c b/revision.c index 4336281286..a7e2339064 100644 --- a/revision.c +++ b/revision.c @@ -153,7 +153,7 @@ static void paths_and_oids_insert(struct hashmap *map, hashmap_entry_init(&entry->ent, hash); entry->path = xstrdup(key.path); oidset_init(&entry->trees, 16); - hashmap_put(map, entry); + hashmap_put(map, &entry->ent); } oidset_insert(&entry->trees, oid); diff --git a/sequencer.c b/sequencer.c index ee11cda7e7..b4ef70e260 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5254,7 +5254,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list) entry->i = i; hashmap_entry_init(&entry->entry, strhash(entry->subject)); - hashmap_put(&subject2item, entry); + hashmap_put(&subject2item, &entry->entry); } } diff --git a/submodule-config.c b/submodule-config.c index 7486745a6a..9248c5ea5b 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -125,7 +125,7 @@ static void cache_put_path(struct submodule_cache *cache, struct submodule_entry *e = xmalloc(sizeof(*e)); hashmap_entry_init(&e->ent, hash); e->config = submodule; - hashmap_put(&cache->for_path, e); + hashmap_put(&cache->for_path, &e->ent); } static void cache_remove_path(struct submodule_cache *cache, diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index 49e715f1cd..de2bd083b9 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -187,7 +187,7 @@ int cmd__hashmap(int argc, const char **argv) entry = alloc_test_entry(hash, p1, p2); /* add / replace entry */ - entry = hashmap_put(&map, entry); + entry = hashmap_put(&map, &entry->ent); /* print and free replaced entry, if any */ puts(entry ? get_value(entry) : "NULL"); From 973d5eea7455e1053842f7474c8ec34755f3525b Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:33 +0000 Subject: [PATCH 10/20] introduce container_of macro This macro is popular within the Linux kernel for supporting intrusive data structures such as linked lists, red-black trees, and chained hash tables while allowing the compiler to do type checking. Later patches will use container_of() to remove the limitation of "hashmap_entry" being location-dependent. This will complete the transition to compile-time type checking for the hashmap API. This macro already exists in our source as "list_entry" in list.h and making "list_entry" an alias to "container_of" as the Linux kernel has done is a possibility. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- git-compat-util.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 83be89de0a..4cc2c8283a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1312,4 +1312,14 @@ void unleak_memory(const void *ptr, size_t len); */ #include "banned.h" +/* + * container_of - Get the address of an object containing a field. + * + * @ptr: pointer to the field. + * @type: type of the object. + * @member: name of the field within the object. + */ +#define container_of(ptr, type, member) \ + ((type *) ((char *)(ptr) - offsetof(type, member))) + #endif From 6bcbdfb277bdc81b5ad6996b3fb005382a35c2ee Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:34 +0000 Subject: [PATCH 11/20] hashmap_get_next returns "struct hashmap_entry *" This is a step towards removing the requirement for hashmap_entry being the first field of a struct. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- diff.c | 19 ++++++++++++------- diffcore-rename.c | 11 +++++++---- hashmap.c | 2 +- hashmap.h | 12 ++++++++---- name-hash.c | 8 +++++--- t/helper/test-hashmap.c | 10 ++++++---- 6 files changed, 39 insertions(+), 23 deletions(-) diff --git a/diff.c b/diff.c index 72d3c6aa19..0978814086 100644 --- a/diff.c +++ b/diff.c @@ -1035,8 +1035,10 @@ static void pmb_advance_or_null_multi_match(struct diff_options *o, { int i; char *got_match = xcalloc(1, pmb_nr); + struct hashmap_entry *ent; - for (; match; match = hashmap_get_next(hm, &match->ent)) { + for (ent = &match->ent; ent; ent = hashmap_get_next(hm, ent)) { + match = container_of(ent, struct moved_entry, ent); for (i = 0; i < pmb_nr; i++) { struct moved_entry *prev = pmb[i].match; struct moved_entry *cur = (prev && prev->next_line) ? @@ -1135,8 +1137,9 @@ static void mark_color_as_moved(struct diff_options *o, for (n = 0; n < o->emitted_symbols->nr; n++) { struct hashmap *hm = NULL; + struct hashmap_entry *ent = NULL; struct moved_entry *key; - struct moved_entry *match = NULL; + struct moved_entry *match; struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n]; enum diff_symbol last_symbol = 0; @@ -1144,20 +1147,20 @@ static void mark_color_as_moved(struct diff_options *o, case DIFF_SYMBOL_PLUS: hm = del_lines; key = prepare_entry(o, n); - match = hashmap_get(hm, &key->ent, NULL); + ent = hashmap_get(hm, &key->ent, NULL); free(key); break; case DIFF_SYMBOL_MINUS: hm = add_lines; key = prepare_entry(o, n); - match = hashmap_get(hm, &key->ent, NULL); + ent = hashmap_get(hm, &key->ent, NULL); free(key); break; default: flipped_block = 0; } - if (!match) { + if (!ent) { int i; adjust_last_block(o, n, block_length); @@ -1169,6 +1172,7 @@ static void mark_color_as_moved(struct diff_options *o, last_symbol = l->s; continue; } + match = container_of(ent, struct moved_entry, ent); if (o->color_moved == COLOR_MOVED_PLAIN) { last_symbol = l->s; @@ -1189,8 +1193,9 @@ static void mark_color_as_moved(struct diff_options *o, * The current line is the start of a new block. * Setup the set of potential blocks. */ - for (; match; match = hashmap_get_next(hm, - &match->ent)) { + for (; ent; ent = hashmap_get_next(hm, ent)) { + match = container_of(ent, struct moved_entry, + ent); ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc); if (o->color_moved_ws_handling & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) { diff --git a/diffcore-rename.c b/diffcore-rename.c index 4670a40179..71aa240a68 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -274,7 +274,7 @@ static int find_identical_files(struct hashmap *srcs, struct diff_options *options) { int renames = 0; - + struct hashmap_entry *ent; struct diff_filespec *target = rename_dst[dst_index].two; struct file_similarity *p, *best = NULL; int i = 100, best_score = -1; @@ -282,12 +282,15 @@ static int find_identical_files(struct hashmap *srcs, /* * Find the best source match for specified destination. */ - p = hashmap_get_from_hash(srcs, + ent = hashmap_get_from_hash(srcs, hash_filespec(options->repo, target), NULL); - for (; p; p = hashmap_get_next(srcs, &p->entry)) { + for (; ent; ent = hashmap_get_next(srcs, ent)) { int score; - struct diff_filespec *source = p->filespec; + struct diff_filespec *source; + + p = container_of(ent, struct file_similarity, entry); + source = p->filespec; /* False hash collision? */ if (!oideq(&source->oid, &target->oid)) diff --git a/hashmap.c b/hashmap.c index 9b83e73d03..22bc7c5b3b 100644 --- a/hashmap.c +++ b/hashmap.c @@ -192,7 +192,7 @@ void *hashmap_get(const struct hashmap *map, const struct hashmap_entry *key, return *find_entry_ptr(map, key, keydata); } -void *hashmap_get_next(const struct hashmap *map, +struct hashmap_entry *hashmap_get_next(const struct hashmap *map, const struct hashmap_entry *entry) { struct hashmap_entry *e = entry->next; diff --git a/hashmap.h b/hashmap.h index d122c75d0f..21bc8aff2b 100644 --- a/hashmap.h +++ b/hashmap.h @@ -55,15 +55,19 @@ * * if (!strcmp("print_all_by_key", action)) { * struct long2string k, *e; + * struct hashmap_entry *ent; * hashmap_entry_init(&k->ent, memhash(&key, sizeof(long))); * k.key = key; * * flags &= ~COMPARE_VALUE; - * e = hashmap_get(&map, &k, NULL); - * if (e) { + * ent = hashmap_get(&map, &k, NULL); + * if (ent) { + * e = container_of(ent, struct long2string, ent); * printf("first: %ld %s\n", e->key, e->value); - * while ((e = hashmap_get_next(&map, e))) + * while ((ent = hashmap_get_next(&map, ent))) { + * e = container_of(ent, struct long2string, ent); * printf("found more: %ld %s\n", e->key, e->value); + * } * } * } * @@ -320,7 +324,7 @@ static inline void *hashmap_get_from_hash(const struct hashmap *map, * `entry` is the hashmap_entry to start the search from, obtained via a previous * call to `hashmap_get` or `hashmap_get_next`. */ -void *hashmap_get_next(const struct hashmap *map, +struct hashmap_entry *hashmap_get_next(const struct hashmap *map, const struct hashmap_entry *entry); /* diff --git a/name-hash.c b/name-hash.c index 44d788f1ce..dbacb34f50 100644 --- a/name-hash.c +++ b/name-hash.c @@ -702,15 +702,17 @@ void adjust_dirname_case(struct index_state *istate, char *name) struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int icase) { struct cache_entry *ce; + struct hashmap_entry *ent; lazy_init_name_hash(istate); - ce = hashmap_get_from_hash(&istate->name_hash, + ent = hashmap_get_from_hash(&istate->name_hash, memihash(name, namelen), NULL); - while (ce) { + while (ent) { + ce = container_of(ent, struct cache_entry, ent); if (same_name(ce, name, namelen, icase)) return ce; - ce = hashmap_get_next(&istate->name_hash, &ce->ent); + ent = hashmap_get_next(&istate->name_hash, ent); } return NULL; } diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index de2bd083b9..d85b8dc58e 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -194,16 +194,18 @@ int cmd__hashmap(int argc, const char **argv) free(entry); } else if (!strcmp("get", cmd) && p1) { + struct hashmap_entry *e; /* lookup entry in hashmap */ - entry = hashmap_get_from_hash(&map, hash, p1); + e = hashmap_get_from_hash(&map, hash, p1); /* print result */ - if (!entry) + if (!e) puts("NULL"); - while (entry) { + while (e) { + entry = container_of(e, struct test_entry, ent); puts(get_value(entry)); - entry = hashmap_get_next(&map, &entry->ent); + e = hashmap_get_next(&map, e); } } else if (!strcmp("remove", cmd) && p1) { From f0e63c41139f8982add435536d39aff6f3d4ca98 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:35 +0000 Subject: [PATCH 12/20] hashmap: use *_entry APIs to wrap container_of Using `container_of' can be verbose and choosing names for intermediate "struct hashmap_entry" pointers is a hard problem. So introduce "*_entry" APIs inspired by similar linked-list APIs in the Linux kernel. Unfortunately, `__typeof__' is not portable C, so we need an extra parameter to specify the type. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- diff.c | 21 +++++++++------------ diffcore-rename.c | 14 +++++--------- git-compat-util.h | 15 +++++++++++++++ hashmap.h | 40 ++++++++++++++++++++++++++++++++++------ name-hash.c | 11 +++++------ t/helper/test-hashmap.c | 12 +++++------- 6 files changed, 73 insertions(+), 40 deletions(-) diff --git a/diff.c b/diff.c index 0978814086..66cdf4e9ca 100644 --- a/diff.c +++ b/diff.c @@ -1035,10 +1035,8 @@ static void pmb_advance_or_null_multi_match(struct diff_options *o, { int i; char *got_match = xcalloc(1, pmb_nr); - struct hashmap_entry *ent; - for (ent = &match->ent; ent; ent = hashmap_get_next(hm, ent)) { - match = container_of(ent, struct moved_entry, ent); + hashmap_for_each_entry_from(hm, match, struct moved_entry, ent) { for (i = 0; i < pmb_nr; i++) { struct moved_entry *prev = pmb[i].match; struct moved_entry *cur = (prev && prev->next_line) ? @@ -1137,9 +1135,8 @@ static void mark_color_as_moved(struct diff_options *o, for (n = 0; n < o->emitted_symbols->nr; n++) { struct hashmap *hm = NULL; - struct hashmap_entry *ent = NULL; struct moved_entry *key; - struct moved_entry *match; + struct moved_entry *match = NULL; struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n]; enum diff_symbol last_symbol = 0; @@ -1147,20 +1144,22 @@ static void mark_color_as_moved(struct diff_options *o, case DIFF_SYMBOL_PLUS: hm = del_lines; key = prepare_entry(o, n); - ent = hashmap_get(hm, &key->ent, NULL); + match = hashmap_get_entry(hm, key, NULL, + struct moved_entry, ent); free(key); break; case DIFF_SYMBOL_MINUS: hm = add_lines; key = prepare_entry(o, n); - ent = hashmap_get(hm, &key->ent, NULL); + match = hashmap_get_entry(hm, key, NULL, + struct moved_entry, ent); free(key); break; default: flipped_block = 0; } - if (!ent) { + if (!match) { int i; adjust_last_block(o, n, block_length); @@ -1172,7 +1171,6 @@ static void mark_color_as_moved(struct diff_options *o, last_symbol = l->s; continue; } - match = container_of(ent, struct moved_entry, ent); if (o->color_moved == COLOR_MOVED_PLAIN) { last_symbol = l->s; @@ -1193,9 +1191,8 @@ static void mark_color_as_moved(struct diff_options *o, * The current line is the start of a new block. * Setup the set of potential blocks. */ - for (; ent; ent = hashmap_get_next(hm, ent)) { - match = container_of(ent, struct moved_entry, - ent); + hashmap_for_each_entry_from(hm, match, + struct moved_entry, ent) { ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc); if (o->color_moved_ws_handling & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) { diff --git a/diffcore-rename.c b/diffcore-rename.c index 71aa240a68..611b08f463 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -274,23 +274,19 @@ static int find_identical_files(struct hashmap *srcs, struct diff_options *options) { int renames = 0; - struct hashmap_entry *ent; struct diff_filespec *target = rename_dst[dst_index].two; struct file_similarity *p, *best = NULL; int i = 100, best_score = -1; + unsigned int hash = hash_filespec(options->repo, target); /* * Find the best source match for specified destination. */ - ent = hashmap_get_from_hash(srcs, - hash_filespec(options->repo, target), - NULL); - for (; ent; ent = hashmap_get_next(srcs, ent)) { + p = hashmap_get_entry_from_hash(srcs, hash, NULL, + struct file_similarity, entry); + hashmap_for_each_entry_from(srcs, p, struct file_similarity, entry) { int score; - struct diff_filespec *source; - - p = container_of(ent, struct file_similarity, entry); - source = p->filespec; + struct diff_filespec *source = p->filespec; /* False hash collision? */ if (!oideq(&source->oid, &target->oid)) diff --git a/git-compat-util.h b/git-compat-util.h index 4cc2c8283a..4a23b9090b 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1322,4 +1322,19 @@ void unleak_memory(const void *ptr, size_t len); #define container_of(ptr, type, member) \ ((type *) ((char *)(ptr) - offsetof(type, member))) +/* + * helper function for `container_of_or_null' to avoid multiple + * evaluation of @ptr + */ +static inline void *container_of_or_null_offset(void *ptr, size_t offset) +{ + return ptr ? (char *)ptr - offset : NULL; +} + +/* + * like `container_of', but allows returned value to be NULL + */ +#define container_of_or_null(ptr, type, member) \ + (type *)container_of_or_null_offset(ptr, offsetof(type, member)) + #endif diff --git a/hashmap.h b/hashmap.h index 21bc8aff2b..cd42dcc15c 100644 --- a/hashmap.h +++ b/hashmap.h @@ -55,17 +55,15 @@ * * if (!strcmp("print_all_by_key", action)) { * struct long2string k, *e; - * struct hashmap_entry *ent; * hashmap_entry_init(&k->ent, memhash(&key, sizeof(long))); * k.key = key; * * flags &= ~COMPARE_VALUE; - * ent = hashmap_get(&map, &k, NULL); - * if (ent) { - * e = container_of(ent, struct long2string, ent); + * e = hashmap_get_entry(&map, &k, NULL, struct long2string, ent); + * if (e) { * printf("first: %ld %s\n", e->key, e->value); - * while ((ent = hashmap_get_next(&map, ent))) { - * e = container_of(ent, struct long2string, ent); + * while ((e = hashmap_get_next_entry(&map, e, + * struct long2string, ent))) { * printf("found more: %ld %s\n", e->key, e->value); * } * } @@ -387,6 +385,36 @@ static inline void *hashmap_iter_first(struct hashmap *map, return hashmap_iter_next(iter); } +/* + * returns a @pointer of @type matching @keyvar, or NULL if nothing found. + * @keyvar is a pointer of @type + * @member is the name of the "struct hashmap_entry" field in @type + */ +#define hashmap_get_entry(map, keyvar, keydata, type, member) \ + container_of_or_null(hashmap_get(map, &(keyvar)->member, keydata), \ + type, member) + +#define hashmap_get_entry_from_hash(map, hash, keydata, type, member) \ + container_of_or_null(hashmap_get_from_hash(map, hash, keydata), \ + type, member) +/* + * returns the next equal @type pointer to @var, or NULL if not found. + * @var is a pointer of @type + * @member is the name of the "struct hashmap_entry" field in @type + */ +#define hashmap_get_next_entry(map, var, type, member) \ + container_of_or_null(hashmap_get_next(map, &(var)->member), \ + type, member) + +/* + * iterate @map starting from @var, where @var is a pointer of @type + * and @member is the name of the "struct hashmap_entry" field in @type + */ +#define hashmap_for_each_entry_from(map, var, type, member) \ + for (; \ + var; \ + var = hashmap_get_next_entry(map, var, type, member)) + /* * Disable item counting and automatic rehashing when adding/removing items. * diff --git a/name-hash.c b/name-hash.c index dbacb34f50..73b83adf3d 100644 --- a/name-hash.c +++ b/name-hash.c @@ -702,17 +702,16 @@ void adjust_dirname_case(struct index_state *istate, char *name) struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int icase) { struct cache_entry *ce; - struct hashmap_entry *ent; + unsigned int hash = memihash(name, namelen); lazy_init_name_hash(istate); - ent = hashmap_get_from_hash(&istate->name_hash, - memihash(name, namelen), NULL); - while (ent) { - ce = container_of(ent, struct cache_entry, ent); + ce = hashmap_get_entry_from_hash(&istate->name_hash, hash, NULL, + struct cache_entry, ent); + hashmap_for_each_entry_from(&istate->name_hash, ce, + struct cache_entry, ent) { if (same_name(ce, name, namelen, icase)) return ce; - ent = hashmap_get_next(&istate->name_hash, ent); } return NULL; } diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index d85b8dc58e..e82cbfdee2 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -194,18 +194,16 @@ int cmd__hashmap(int argc, const char **argv) free(entry); } else if (!strcmp("get", cmd) && p1) { - struct hashmap_entry *e; - /* lookup entry in hashmap */ - e = hashmap_get_from_hash(&map, hash, p1); + entry = hashmap_get_entry_from_hash(&map, hash, p1, + struct test_entry, ent); /* print result */ - if (!e) + if (!entry) puts("NULL"); - while (e) { - entry = container_of(e, struct test_entry, ent); + hashmap_for_each_entry_from(&map, entry, + struct test_entry, ent) { puts(get_value(entry)); - e = hashmap_get_next(&map, e); } } else if (!strcmp("remove", cmd) && p1) { From f23a465132a22860684ac66052cf9a954a18e27d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:36 +0000 Subject: [PATCH 13/20] hashmap_get{,_from_hash} return "struct hashmap_entry *" Update callers to use hashmap_get_entry, hashmap_get_entry_from_hash or container_of as appropriate. This is another step towards eliminating the requirement of hashmap_entry being the first field in a struct. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 2 +- blame.c | 11 ++++++++--- builtin/describe.c | 3 ++- builtin/difftool.c | 2 +- builtin/fast-export.c | 2 +- builtin/fetch.c | 11 +++++++---- config.c | 3 ++- hashmap.c | 7 ++++--- hashmap.h | 12 +++++++----- merge-recursive.c | 6 ++++-- name-hash.c | 3 ++- packfile.c | 5 +++-- patch-ids.c | 3 ++- ref-filter.c | 12 +++++++----- refs.c | 5 ++++- remote.c | 7 ++++--- revision.c | 3 ++- sequencer.c | 7 +++++-- sub-process.c | 3 ++- submodule-config.c | 6 ++++-- 20 files changed, 72 insertions(+), 41 deletions(-) diff --git a/attr.c b/attr.c index 9bdef61cc3..4230bee63d 100644 --- a/attr.c +++ b/attr.c @@ -101,7 +101,7 @@ static void *attr_hashmap_get(struct attr_hashmap *map, hashmap_entry_init(&k.ent, memhash(key, keylen)); k.key = key; k.keylen = keylen; - e = hashmap_get(&map->map, &k.ent, NULL); + e = hashmap_get_entry(&map->map, &k, NULL, struct attr_hash_entry, ent); return e ? e->value : NULL; } diff --git a/blame.c b/blame.c index 00f8f3fb0a..aa46c7ec52 100644 --- a/blame.c +++ b/blame.c @@ -419,7 +419,8 @@ static void get_fingerprint(struct fingerprint *result, continue; hashmap_entry_init(&entry->entry, hash); - found_entry = hashmap_get(&result->map, &entry->entry, NULL); + found_entry = hashmap_get_entry(&result->map, entry, NULL, + struct fingerprint_entry, entry); if (found_entry) { found_entry->count += 1; } else { @@ -452,7 +453,9 @@ static int fingerprint_similarity(struct fingerprint *a, struct fingerprint *b) hashmap_iter_init(&b->map, &iter); while ((entry_b = hashmap_iter_next(&iter))) { - if ((entry_a = hashmap_get(&a->map, &entry_b->entry, NULL))) { + entry_a = hashmap_get_entry(&a->map, entry_b, NULL, + struct fingerprint_entry, entry); + if (entry_a) { intersection += entry_a->count < entry_b->count ? entry_a->count : entry_b->count; } @@ -471,7 +474,9 @@ static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b) hashmap_iter_init(&b->map, &iter); while ((entry_b = hashmap_iter_next(&iter))) { - if ((entry_a = hashmap_get(&a->map, &entry_b->entry, NULL))) { + entry_a = hashmap_get_entry(&a->map, entry_b, NULL, + struct fingerprint_entry, entry); + if (entry_a) { if (entry_a->count <= entry_b->count) hashmap_remove(&a->map, &entry_b->entry, NULL); else diff --git a/builtin/describe.c b/builtin/describe.c index f5e0a7e033..c6d2386b64 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -76,7 +76,8 @@ static int commit_name_neq(const void *unused_cmp_data, static inline struct commit_name *find_commit_name(const struct object_id *peeled) { - return hashmap_get_from_hash(&names, oidhash(peeled), peeled); + return hashmap_get_entry_from_hash(&names, oidhash(peeled), peeled, + struct commit_name, entry); } static int replace_name(struct commit_name *e, diff --git a/builtin/difftool.c b/builtin/difftool.c index f41298d199..fa9c862e3a 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -162,7 +162,7 @@ static void add_left_or_right(struct hashmap *map, const char *path, FLEX_ALLOC_STR(e, path, path); hashmap_entry_init(&e->entry, strhash(path)); - existing = hashmap_get(map, &e->entry, NULL); + existing = hashmap_get_entry(map, e, NULL, struct pair_entry, entry); if (existing) { free(e); e = existing; diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 192e21dae4..25195badd4 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -151,7 +151,7 @@ static const void *anonymize_mem(struct hashmap *map, hashmap_entry_init(&key.hash, memhash(orig, *len)); key.orig = orig; key.orig_len = *len; - ret = hashmap_get(map, &key.hash, NULL); + ret = hashmap_get_entry(map, &key, NULL, struct anonymized_entry, hash); if (!ret) { ret = xmalloc(sizeof(*ret)); diff --git a/builtin/fetch.c b/builtin/fetch.c index 909dbde909..d06f2b98aa 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -383,8 +383,10 @@ static void find_non_local_tags(const struct ref *refs, for_each_string_list_item(remote_ref_item, &remote_refs_list) { const char *refname = remote_ref_item->string; struct ref *rm; + unsigned int hash = strhash(refname); - item = hashmap_get_from_hash(&remote_refs, strhash(refname), refname); + item = hashmap_get_entry_from_hash(&remote_refs, hash, refname, + struct refname_hash_entry, ent); if (!item) BUG("unseen remote ref?"); @@ -516,10 +518,11 @@ static struct ref *get_ref_map(struct remote *remote, if (rm->peer_ref) { const char *refname = rm->peer_ref->name; struct refname_hash_entry *peer_item; + unsigned int hash = strhash(refname); - peer_item = hashmap_get_from_hash(&existing_refs, - strhash(refname), - refname); + peer_item = hashmap_get_entry_from_hash(&existing_refs, + hash, refname, + struct refname_hash_entry, ent); if (peer_item) { struct object_id *old_oid = &peer_item->oid; oidcpy(&rm->peer_ref->old_oid, old_oid); diff --git a/config.c b/config.c index 1a1b6675fd..4952d1cc9e 100644 --- a/config.c +++ b/config.c @@ -1863,7 +1863,8 @@ static struct config_set_element *configset_find_element(struct config_set *cs, hashmap_entry_init(&k.ent, strhash(normalized_key)); k.key = normalized_key; - found_entry = hashmap_get(&cs->config_hash, &k.ent, NULL); + found_entry = hashmap_get_entry(&cs->config_hash, &k, NULL, + struct config_set_element, ent); free(normalized_key); return found_entry; } diff --git a/hashmap.c b/hashmap.c index 22bc7c5b3b..5662bee10a 100644 --- a/hashmap.c +++ b/hashmap.c @@ -186,8 +186,9 @@ void hashmap_free(struct hashmap *map, int free_entries) memset(map, 0, sizeof(*map)); } -void *hashmap_get(const struct hashmap *map, const struct hashmap_entry *key, - const void *keydata) +struct hashmap_entry *hashmap_get(const struct hashmap *map, + const struct hashmap_entry *key, + const void *keydata) { return *find_entry_ptr(map, key, keydata); } @@ -298,7 +299,7 @@ const void *memintern(const void *data, size_t len) /* lookup interned string in pool */ hashmap_entry_init(&key.ent, memhash(data, len)); key.len = len; - e = hashmap_get(&map, &key.ent, data); + e = hashmap_get_entry(&map, &key, data, struct pool_entry, ent); if (!e) { /* not found: create it */ FLEX_ALLOC_MEM(e, data, data, len); diff --git a/hashmap.h b/hashmap.h index cd42dcc15c..82ddb0ef41 100644 --- a/hashmap.h +++ b/hashmap.h @@ -290,8 +290,9 @@ static inline unsigned int hashmap_get_size(struct hashmap *map) * If an entry with matching hash code is found, `key` and `keydata` are passed * to `hashmap_cmp_fn` to decide whether the entry matches the key. */ -void *hashmap_get(const struct hashmap *map, const struct hashmap_entry *key, - const void *keydata); +struct hashmap_entry *hashmap_get(const struct hashmap *map, + const struct hashmap_entry *key, + const void *keydata); /* * Returns the hashmap entry for the specified hash code and key data, @@ -305,9 +306,10 @@ void *hashmap_get(const struct hashmap *map, const struct hashmap_entry *key, * `entry_or_key` parameter of `hashmap_cmp_fn` points to a hashmap_entry * structure that should not be used in the comparison. */ -static inline void *hashmap_get_from_hash(const struct hashmap *map, - unsigned int hash, - const void *keydata) +static inline struct hashmap_entry *hashmap_get_from_hash( + const struct hashmap *map, + unsigned int hash, + const void *keydata) { struct hashmap_entry key; hashmap_entry_init(&key, hash); diff --git a/merge-recursive.c b/merge-recursive.c index a685b4fb69..8274828c4d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -63,7 +63,8 @@ static struct dir_rename_entry *dir_rename_find_entry(struct hashmap *hashmap, return NULL; hashmap_entry_init(&key.ent, strhash(dir)); key.dir = dir; - return hashmap_get(hashmap, &key.ent, NULL); + return hashmap_get_entry(hashmap, &key, NULL, + struct dir_rename_entry, ent); } static int dir_rename_cmp(const void *unused_cmp_data, @@ -99,7 +100,8 @@ static struct collision_entry *collision_find_entry(struct hashmap *hashmap, hashmap_entry_init(&key.ent, strhash(target_file)); key.target_file = target_file; - return hashmap_get(hashmap, &key.ent, NULL); + return hashmap_get_entry(hashmap, &key, NULL, + struct collision_entry, ent); } static int collision_cmp(void *unused_cmp_data, diff --git a/name-hash.c b/name-hash.c index 73b83adf3d..aa8253ddd5 100644 --- a/name-hash.c +++ b/name-hash.c @@ -35,7 +35,8 @@ static struct dir_entry *find_dir_entry__hash(struct index_state *istate, struct dir_entry key; hashmap_entry_init(&key.ent, hash); key.namelen = namelen; - return hashmap_get(&istate->dir_hash, &key.ent, name); + return hashmap_get_entry(&istate->dir_hash, &key, name, + struct dir_entry, ent); } static struct dir_entry *find_dir_entry(struct index_state *istate, diff --git a/packfile.c b/packfile.c index 3edd648de0..f2aa34bb49 100644 --- a/packfile.c +++ b/packfile.c @@ -1381,7 +1381,7 @@ static unsigned int pack_entry_hash(struct packed_git *p, off_t base_offset) static struct delta_base_cache_entry * get_delta_base_cache_entry(struct packed_git *p, off_t base_offset) { - struct hashmap_entry entry; + struct hashmap_entry entry, *e; struct delta_base_cache_key key; if (!delta_base_cache.cmpfn) @@ -1390,7 +1390,8 @@ get_delta_base_cache_entry(struct packed_git *p, off_t base_offset) hashmap_entry_init(&entry, pack_entry_hash(p, base_offset)); key.p = p; key.base_offset = base_offset; - return hashmap_get(&delta_base_cache, &entry, &key); + e = hashmap_get(&delta_base_cache, &entry, &key); + return e ? container_of(e, struct delta_base_cache_entry, ent) : NULL; } static int delta_base_cache_key_eq(const struct delta_base_cache_key *a, diff --git a/patch-ids.c b/patch-ids.c index 437f29e42c..176c47d967 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -99,7 +99,8 @@ struct patch_id *has_commit_patch_id(struct commit *commit, if (init_patch_id_entry(&patch, commit, ids)) return NULL; - return hashmap_get(&ids->patches, &patch.ent, NULL); + return hashmap_get_entry(&ids->patches, &patch, NULL, + struct patch_id, ent); } struct patch_id *add_commit_patch_id(struct commit *commit, diff --git a/ref-filter.c b/ref-filter.c index d939ebc6bb..9999426914 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1585,18 +1585,20 @@ static void lazy_init_worktree_map(void) static char *get_worktree_path(const struct used_atom *atom, const struct ref_array_item *ref) { - struct hashmap_entry entry; + struct hashmap_entry entry, *e; struct ref_to_worktree_entry *lookup_result; lazy_init_worktree_map(); hashmap_entry_init(&entry, strhash(ref->refname)); - lookup_result = hashmap_get(&(ref_to_worktree_map.map), &entry, ref->refname); + e = hashmap_get(&(ref_to_worktree_map.map), &entry, ref->refname); - if (lookup_result) - return xstrdup(lookup_result->wt->path); - else + if (!e) return xstrdup(""); + + lookup_result = container_of(e, struct ref_to_worktree_entry, ent); + + return xstrdup(lookup_result->wt->path); } /* diff --git a/refs.c b/refs.c index 3e55031256..43a95105f1 100644 --- a/refs.c +++ b/refs.c @@ -1815,12 +1815,15 @@ static struct ref_store *lookup_ref_store_map(struct hashmap *map, const char *name) { struct ref_store_hash_entry *entry; + unsigned int hash; if (!map->tablesize) /* It's initialized on demand in register_ref_store(). */ return NULL; - entry = hashmap_get_from_hash(map, strhash(name), name); + hash = strhash(name); + entry = hashmap_get_entry_from_hash(map, hash, name, + struct ref_store_hash_entry, ent); return entry ? entry->refs : NULL; } diff --git a/remote.c b/remote.c index 8ca23d95dc..ed95ae6ed6 100644 --- a/remote.c +++ b/remote.c @@ -135,7 +135,7 @@ static struct remote *make_remote(const char *name, int len) { struct remote *ret, *replaced; struct remotes_hash_key lookup; - struct hashmap_entry lookup_entry; + struct hashmap_entry lookup_entry, *e; if (!len) len = strlen(name); @@ -145,8 +145,9 @@ static struct remote *make_remote(const char *name, int len) lookup.len = len; hashmap_entry_init(&lookup_entry, memhash(name, len)); - if ((ret = hashmap_get(&remotes_hash, &lookup_entry, &lookup)) != NULL) - return ret; + e = hashmap_get(&remotes_hash, &lookup_entry, &lookup); + if (e) + return container_of(e, struct remote, ent); ret = xcalloc(1, sizeof(struct remote)); ret->prune = -1; /* unspecified */ diff --git a/revision.c b/revision.c index a7e2339064..d5f534209d 100644 --- a/revision.c +++ b/revision.c @@ -147,7 +147,8 @@ static void paths_and_oids_insert(struct hashmap *map, key.path = (char *)path; oidset_init(&key.trees, 0); - entry = hashmap_get(map, &key.ent, NULL); + entry = hashmap_get_entry(map, &key, NULL, + struct path_and_oids_entry, ent); if (!entry) { entry = xcalloc(1, sizeof(struct path_and_oids_entry)); hashmap_entry_init(&entry->ent, hash); diff --git a/sequencer.c b/sequencer.c index b4ef70e260..aea2cb12cc 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5217,8 +5217,11 @@ int todo_list_rearrange_squash(struct todo_list *todo_list) break; } - if ((entry = hashmap_get_from_hash(&subject2item, - strhash(p), p))) + entry = hashmap_get_entry_from_hash(&subject2item, + strhash(p), p, + struct subject2item_entry, + entry); + if (entry) /* found by title */ i2 = entry->i; else if (!strchr(p, ' ') && diff --git a/sub-process.c b/sub-process.c index 99fccef592..f2fcc16c3e 100644 --- a/sub-process.c +++ b/sub-process.c @@ -22,7 +22,8 @@ struct subprocess_entry *subprocess_find_entry(struct hashmap *hashmap, const ch hashmap_entry_init(&key.ent, strhash(cmd)); key.cmd = cmd; - return hashmap_get(hashmap, &key.ent, NULL); + return hashmap_get_entry(hashmap, &key, NULL, + struct subprocess_entry, ent); } int subprocess_read_status(int fd, struct strbuf *status) diff --git a/submodule-config.c b/submodule-config.c index 9248c5ea5b..b031884789 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -166,7 +166,8 @@ static const struct submodule *cache_lookup_path(struct submodule_cache *cache, hashmap_entry_init(&key.ent, hash); key.config = &key_config; - entry = hashmap_get(&cache->for_path, &key.ent, NULL); + entry = hashmap_get_entry(&cache->for_path, &key, NULL, + struct submodule_entry, ent); if (entry) return entry->config; return NULL; @@ -186,7 +187,8 @@ static struct submodule *cache_lookup_name(struct submodule_cache *cache, hashmap_entry_init(&key.ent, hash); key.config = &key_config; - entry = hashmap_get(&cache->for_name, &key.ent, NULL); + entry = hashmap_get_entry(&cache->for_name, &key, NULL, + struct submodule_entry, ent); if (entry) return entry->config; return NULL; From 939af16eac1608766273d3971598dbcc4fe09928 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:37 +0000 Subject: [PATCH 14/20] hashmap_cmp_fn takes hashmap_entry params Another step in eliminating the requirement of hashmap_entry being the first member of a struct. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 10 ++++++---- builtin/describe.c | 10 ++++++---- builtin/difftool.c | 31 +++++++++++++++++++------------ builtin/fast-export.c | 9 +++++++-- builtin/fetch.c | 9 +++++---- config.c | 10 ++++++---- diff.c | 12 +++++++----- hashmap.c | 17 +++++++++++------ hashmap.h | 13 +++++++++---- merge-recursive.c | 33 +++++++++++++++++++++------------ name-hash.c | 21 +++++++++++++-------- oidmap.c | 14 +++++++++----- packfile.c | 9 +++++++-- patch-ids.c | 10 ++++++---- ref-filter.c | 11 +++++++---- refs.c | 11 ++++++++--- remote.c | 10 ++++++---- revision.c | 11 ++++++++--- sequencer.c | 24 +++++++++++++++++------- sub-process.c | 10 ++++++---- sub-process.h | 4 ++-- submodule-config.c | 20 ++++++++++++-------- t/helper/test-hashmap.c | 10 ++++++---- 23 files changed, 204 insertions(+), 115 deletions(-) diff --git a/attr.c b/attr.c index 4230bee63d..6053481610 100644 --- a/attr.c +++ b/attr.c @@ -70,12 +70,14 @@ struct attr_hash_entry { /* attr_hashmap comparison function */ static int attr_hash_entry_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct attr_hash_entry *a = entry; - const struct attr_hash_entry *b = entry_or_key; + const struct attr_hash_entry *a, *b; + + a = container_of(eptr, const struct attr_hash_entry, ent); + b = container_of(entry_or_key, const struct attr_hash_entry, ent); return (a->keylen != b->keylen) || strncmp(a->key, b->key, a->keylen); } diff --git a/builtin/describe.c b/builtin/describe.c index c6d2386b64..e9267b5c9c 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -64,12 +64,14 @@ static const char *prio_names[] = { }; static int commit_name_neq(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *peeled) { - const struct commit_name *cn1 = entry; - const struct commit_name *cn2 = entry_or_key; + const struct commit_name *cn1, *cn2; + + cn1 = container_of(eptr, const struct commit_name, entry); + cn2 = container_of(entry_or_key, const struct commit_name, entry); return !oideq(&cn1->peeled, peeled ? peeled : &cn2->peeled); } diff --git a/builtin/difftool.c b/builtin/difftool.c index fa9c862e3a..4a37b3edee 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -125,12 +125,15 @@ struct working_tree_entry { }; static int working_tree_entry_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct working_tree_entry *a = entry; - const struct working_tree_entry *b = entry_or_key; + const struct working_tree_entry *a, *b; + + a = container_of(eptr, const struct working_tree_entry, entry); + b = container_of(entry_or_key, const struct working_tree_entry, entry); + return strcmp(a->path, b->path); } @@ -145,12 +148,14 @@ struct pair_entry { }; static int pair_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct pair_entry *a = entry; - const struct pair_entry *b = entry_or_key; + const struct pair_entry *a, *b; + + a = container_of(eptr, const struct pair_entry, entry); + b = container_of(entry_or_key, const struct pair_entry, entry); return strcmp(a->path, b->path); } @@ -179,12 +184,14 @@ struct path_entry { }; static int path_entry_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *key) { - const struct path_entry *a = entry; - const struct path_entry *b = entry_or_key; + const struct path_entry *a, *b; + + a = container_of(eptr, const struct path_entry, entry); + b = container_of(entry_or_key, const struct path_entry, entry); return strcmp(a->path, key ? key : b->path); } diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 25195badd4..ef0578bf90 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -126,10 +126,15 @@ struct anonymized_entry { }; static int anonymized_entry_cmp(const void *unused_cmp_data, - const void *va, const void *vb, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct anonymized_entry *a = va, *b = vb; + const struct anonymized_entry *a, *b; + + a = container_of(eptr, const struct anonymized_entry, hash); + b = container_of(entry_or_key, const struct anonymized_entry, hash); + return a->orig_len != b->orig_len || memcmp(a->orig, b->orig, a->orig_len); } diff --git a/builtin/fetch.c b/builtin/fetch.c index d06f2b98aa..476c2416e3 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -258,13 +258,14 @@ struct refname_hash_entry { }; static int refname_hash_entry_cmp(const void *hashmap_cmp_fn_data, - const void *e1_, - const void *e2_, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { - const struct refname_hash_entry *e1 = e1_; - const struct refname_hash_entry *e2 = e2_; + const struct refname_hash_entry *e1, *e2; + e1 = container_of(eptr, const struct refname_hash_entry, ent); + e2 = container_of(entry_or_key, const struct refname_hash_entry, ent); return strcmp(e1->refname, keydata ? keydata : e2->refname); } diff --git a/config.c b/config.c index 4952d1cc9e..33043ee73c 100644 --- a/config.c +++ b/config.c @@ -1914,12 +1914,14 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha } static int config_set_element_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct config_set_element *e1 = entry; - const struct config_set_element *e2 = entry_or_key; + const struct config_set_element *e1, *e2; + + e1 = container_of(eptr, const struct config_set_element, ent); + e2 = container_of(entry_or_key, const struct config_set_element, ent); return strcmp(e1->key, e2->key); } diff --git a/diff.c b/diff.c index 66cdf4e9ca..5eaf689fcc 100644 --- a/diff.c +++ b/diff.c @@ -933,16 +933,18 @@ static int cmp_in_block_with_wsd(const struct diff_options *o, } static int moved_entry_cmp(const void *hashmap_cmp_fn_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { const struct diff_options *diffopt = hashmap_cmp_fn_data; - const struct moved_entry *a = entry; - const struct moved_entry *b = entry_or_key; + const struct moved_entry *a, *b; unsigned flags = diffopt->color_moved_ws_handling & XDF_WHITESPACE_FLAGS; + a = container_of(eptr, const struct moved_entry, ent); + b = container_of(entry_or_key, const struct moved_entry, ent); + if (diffopt->color_moved_ws_handling & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) /* @@ -1019,7 +1021,7 @@ static void pmb_advance_or_null(struct diff_options *o, struct moved_entry *prev = pmb[i].match; struct moved_entry *cur = (prev && prev->next_line) ? prev->next_line : NULL; - if (cur && !hm->cmpfn(o, cur, match, NULL)) { + if (cur && !hm->cmpfn(o, &cur->ent, &match->ent, NULL)) { pmb[i].match = cur; } else { pmb[i].match = NULL; diff --git a/hashmap.c b/hashmap.c index 5662bee10a..64f6accfff 100644 --- a/hashmap.c +++ b/hashmap.c @@ -140,8 +140,8 @@ static inline struct hashmap_entry **find_entry_ptr(const struct hashmap *map, } static int always_equal(const void *unused_cmp_data, - const void *unused1, - const void *unused2, + const struct hashmap_entry *unused1, + const struct hashmap_entry *unused2, const void *unused_keydata) { return 0; @@ -279,10 +279,15 @@ struct pool_entry { }; static int pool_entry_cmp(const void *unused_cmp_data, - const struct pool_entry *e1, - const struct pool_entry *e2, - const unsigned char *keydata) + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *keydata) { + const struct pool_entry *e1, *e2; + + e1 = container_of(eptr, const struct pool_entry, ent); + e2 = container_of(entry_or_key, const struct pool_entry, ent); + return e1->data != keydata && (e1->len != e2->len || memcmp(e1->data, keydata, e1->len)); } @@ -294,7 +299,7 @@ const void *memintern(const void *data, size_t len) /* initialize string pool hashmap */ if (!map.tablesize) - hashmap_init(&map, (hashmap_cmp_fn) pool_entry_cmp, NULL, 0); + hashmap_init(&map, pool_entry_cmp, NULL, 0); /* lookup interned string in pool */ hashmap_entry_init(&key.ent, memhash(data, len)); diff --git a/hashmap.h b/hashmap.h index 82ddb0ef41..8f5c163d56 100644 --- a/hashmap.h +++ b/hashmap.h @@ -21,12 +21,16 @@ * #define COMPARE_VALUE 1 * * static int long2string_cmp(const void *hashmap_cmp_fn_data, - * const struct long2string *e1, - * const struct long2string *e2, + * const struct hashmap_entry *eptr, + * const struct hashmap_entry *entry_or_key, * const void *keydata) * { * const char *string = keydata; * unsigned flags = *(unsigned *)hashmap_cmp_fn_data; + * const struct long2string *e1, *e2; + * + * e1 = container_of(eptr, const struct long2string, ent); + * e2 = container_of(entry_or_key, const struct long2string, ent); * * if (flags & COMPARE_VALUE) * return e1->key != e2->key || @@ -41,7 +45,7 @@ * char value[255], action[32]; * unsigned flags = 0; * - * hashmap_init(&map, (hashmap_cmp_fn) long2string_cmp, &flags, 0); + * hashmap_init(&map, long2string_cmp, &flags, 0); * * while (scanf("%s %ld %s", action, &key, value)) { * @@ -172,7 +176,8 @@ struct hashmap_entry { * The `hashmap_cmp_fn_data` entry is the pointer given in the init function. */ typedef int (*hashmap_cmp_fn)(const void *hashmap_cmp_fn_data, - const void *entry, const void *entry_or_key, + const struct hashmap_entry *entry, + const struct hashmap_entry *entry_or_key, const void *keydata); /* diff --git a/merge-recursive.c b/merge-recursive.c index 8274828c4d..b06e9f7f0b 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -35,14 +35,16 @@ struct path_hashmap_entry { }; static int path_hashmap_cmp(const void *cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { - const struct path_hashmap_entry *a = entry; - const struct path_hashmap_entry *b = entry_or_key; + const struct path_hashmap_entry *a, *b; const char *key = keydata; + a = container_of(eptr, const struct path_hashmap_entry, e); + b = container_of(entry_or_key, const struct path_hashmap_entry, e); + if (ignore_case) return strcasecmp(a->path, key ? key : b->path); else @@ -68,12 +70,14 @@ static struct dir_rename_entry *dir_rename_find_entry(struct hashmap *hashmap, } static int dir_rename_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct dir_rename_entry *e1 = entry; - const struct dir_rename_entry *e2 = entry_or_key; + const struct dir_rename_entry *e1, *e2; + + e1 = container_of(eptr, const struct dir_rename_entry, ent); + e2 = container_of(entry_or_key, const struct dir_rename_entry, ent); return strcmp(e1->dir, e2->dir); } @@ -104,17 +108,22 @@ static struct collision_entry *collision_find_entry(struct hashmap *hashmap, struct collision_entry, ent); } -static int collision_cmp(void *unused_cmp_data, - const struct collision_entry *e1, - const struct collision_entry *e2, +static int collision_cmp(const void *unused_cmp_data, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { + const struct collision_entry *e1, *e2; + + e1 = container_of(eptr, const struct collision_entry, ent); + e2 = container_of(entry_or_key, const struct collision_entry, ent); + return strcmp(e1->target_file, e2->target_file); } static void collision_init(struct hashmap *map) { - hashmap_init(map, (hashmap_cmp_fn) collision_cmp, NULL, 0); + hashmap_init(map, collision_cmp, NULL, 0); } static void flush_output(struct merge_options *opt) diff --git a/name-hash.c b/name-hash.c index aa8253ddd5..85a1ce982c 100644 --- a/name-hash.c +++ b/name-hash.c @@ -17,14 +17,16 @@ struct dir_entry { }; static int dir_entry_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { - const struct dir_entry *e1 = entry; - const struct dir_entry *e2 = entry_or_key; + const struct dir_entry *e1, *e2; const char *name = keydata; + e1 = container_of(eptr, const struct dir_entry, ent); + e2 = container_of(entry_or_key, const struct dir_entry, ent); + return e1->namelen != e2->namelen || strncasecmp(e1->name, name ? name : e2->name, e1->namelen); } @@ -115,12 +117,15 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce) } static int cache_entry_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *remove) { - const struct cache_entry *ce1 = entry; - const struct cache_entry *ce2 = entry_or_key; + const struct cache_entry *ce1, *ce2; + + ce1 = container_of(eptr, const struct cache_entry, ent); + ce2 = container_of(entry_or_key, const struct cache_entry, ent); + /* * For remove_name_hash, find the exact entry (pointer equality); for * index_file_exists, find all entries with matching hash code and diff --git a/oidmap.c b/oidmap.c index cd22b3a8bf..4942599391 100644 --- a/oidmap.c +++ b/oidmap.c @@ -2,14 +2,18 @@ #include "oidmap.h" static int oidmap_neq(const void *hashmap_cmp_fn_data, - const void *entry, const void *entry_or_key, + const struct hashmap_entry *e1, + const struct hashmap_entry *e2, const void *keydata) { - const struct oidmap_entry *entry_ = entry; + const struct oidmap_entry *a, *b; + + a = container_of(e1, const struct oidmap_entry, internal_entry); + b = container_of(e2, const struct oidmap_entry, internal_entry); + if (keydata) - return !oideq(&entry_->oid, (const struct object_id *) keydata); - return !oideq(&entry_->oid, - &((const struct oidmap_entry *) entry_or_key)->oid); + return !oideq(&a->oid, (const struct object_id *) keydata); + return !oideq(&a->oid, &b->oid); } void oidmap_init(struct oidmap *map, size_t initial_size) diff --git a/packfile.c b/packfile.c index f2aa34bb49..675d5f2287 100644 --- a/packfile.c +++ b/packfile.c @@ -1401,11 +1401,16 @@ static int delta_base_cache_key_eq(const struct delta_base_cache_key *a, } static int delta_base_cache_hash_cmp(const void *unused_cmp_data, - const void *va, const void *vb, + const struct hashmap_entry *va, + const struct hashmap_entry *vb, const void *vkey) { - const struct delta_base_cache_entry *a = va, *b = vb; + const struct delta_base_cache_entry *a, *b; const struct delta_base_cache_key *key = vkey; + + a = container_of(va, const struct delta_base_cache_entry, ent); + b = container_of(vb, const struct delta_base_cache_entry, ent); + if (key) return !delta_base_cache_key_eq(&a->key, key); else diff --git a/patch-ids.c b/patch-ids.c index 176c47d967..75f8c9f1a1 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -36,14 +36,16 @@ int commit_patch_id(struct commit *commit, struct diff_options *options, * any significance; only that it is non-zero matters. */ static int patch_id_neq(const void *cmpfn_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { /* NEEDSWORK: const correctness? */ struct diff_options *opt = (void *)cmpfn_data; - struct patch_id *a = (void *)entry; - struct patch_id *b = (void *)entry_or_key; + struct patch_id *a, *b; + + a = container_of(eptr, struct patch_id, ent); + b = container_of(entry_or_key, struct patch_id, ent); if (is_null_oid(&a->patch_id) && commit_patch_id(a->commit, opt, &a->patch_id, 0, 0)) diff --git a/ref-filter.c b/ref-filter.c index 9999426914..4613df8826 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -84,12 +84,15 @@ struct ref_to_worktree_entry { }; static int ref_to_worktree_map_cmpfnc(const void *unused_lookupdata, - const void *existing_hashmap_entry_to_test, - const void *key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *kptr, const void *keydata_aka_refname) { - const struct ref_to_worktree_entry *e = existing_hashmap_entry_to_test; - const struct ref_to_worktree_entry *k = key; + const struct ref_to_worktree_entry *e, *k; + + e = container_of(eptr, const struct ref_to_worktree_entry, ent); + k = container_of(kptr, const struct ref_to_worktree_entry, ent); + return strcmp(e->wt->head_ref, keydata_aka_refname ? keydata_aka_refname : k->wt->head_ref); } diff --git a/refs.c b/refs.c index 43a95105f1..2d3eb40f39 100644 --- a/refs.c +++ b/refs.c @@ -1781,11 +1781,16 @@ struct ref_store_hash_entry }; static int ref_store_hash_cmp(const void *unused_cmp_data, - const void *entry, const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { - const struct ref_store_hash_entry *e1 = entry, *e2 = entry_or_key; - const char *name = keydata ? keydata : e2->name; + const struct ref_store_hash_entry *e1, *e2; + const char *name; + + e1 = container_of(eptr, const struct ref_store_hash_entry, ent); + e2 = container_of(entry_or_key, const struct ref_store_hash_entry, ent); + name = keydata ? keydata : e2->name; return strcmp(e1->name, name); } diff --git a/remote.c b/remote.c index ed95ae6ed6..fa9cadcfbd 100644 --- a/remote.c +++ b/remote.c @@ -111,14 +111,16 @@ struct remotes_hash_key { }; static int remotes_hash_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { - const struct remote *a = entry; - const struct remote *b = entry_or_key; + const struct remote *a, *b; const struct remotes_hash_key *key = keydata; + a = container_of(eptr, const struct remote, ent); + b = container_of(entry_or_key, const struct remote, ent); + if (key) return strncmp(a->name, key->str, key->len) || a->name[key->len]; else diff --git a/revision.c b/revision.c index d5f534209d..f32fbc5e2e 100644 --- a/revision.c +++ b/revision.c @@ -107,16 +107,21 @@ struct path_and_oids_entry { }; static int path_and_oids_cmp(const void *hashmap_cmp_fn_data, - const struct path_and_oids_entry *e1, - const struct path_and_oids_entry *e2, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { + const struct path_and_oids_entry *e1, *e2; + + e1 = container_of(eptr, const struct path_and_oids_entry, ent); + e2 = container_of(entry_or_key, const struct path_and_oids_entry, ent); + return strcmp(e1->path, e2->path); } static void paths_and_oids_init(struct hashmap *map) { - hashmap_init(map, (hashmap_cmp_fn) path_and_oids_cmp, NULL, 0); + hashmap_init(map, path_and_oids_cmp, NULL, 0); } static void paths_and_oids_clear(struct hashmap *map) diff --git a/sequencer.c b/sequencer.c index aea2cb12cc..b3e7319b55 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4440,9 +4440,14 @@ struct labels_entry { char label[FLEX_ARRAY]; }; -static int labels_cmp(const void *fndata, const struct labels_entry *a, - const struct labels_entry *b, const void *key) +static int labels_cmp(const void *fndata, const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *key) { + const struct labels_entry *a, *b; + + a = container_of(eptr, const struct labels_entry, entry); + b = container_of(entry_or_key, const struct labels_entry, entry); + return key ? strcmp(a->label, key) : strcmp(a->label, b->label); } @@ -4573,7 +4578,7 @@ static int make_script_with_merges(struct pretty_print_context *pp, oidmap_init(&commit2todo, 0); oidmap_init(&state.commit2label, 0); - hashmap_init(&state.labels, (hashmap_cmp_fn) labels_cmp, NULL, 0); + hashmap_init(&state.labels, labels_cmp, NULL, 0); strbuf_init(&state.buf, 32); if (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) { @@ -5138,9 +5143,15 @@ struct subject2item_entry { }; static int subject2item_cmp(const void *fndata, - const struct subject2item_entry *a, - const struct subject2item_entry *b, const void *key) + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *key) { + const struct subject2item_entry *a, *b; + + a = container_of(eptr, const struct subject2item_entry, entry); + b = container_of(entry_or_key, const struct subject2item_entry, entry); + return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject); } @@ -5173,8 +5184,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list) * In that case, last[i] will indicate the index of the latest item to * be moved to appear after the i'th. */ - hashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp, - NULL, todo_list->nr); + hashmap_init(&subject2item, subject2item_cmp, NULL, todo_list->nr); ALLOC_ARRAY(next, todo_list->nr); ALLOC_ARRAY(tail, todo_list->nr); ALLOC_ARRAY(subjects, todo_list->nr); diff --git a/sub-process.c b/sub-process.c index f2fcc16c3e..ad94f72665 100644 --- a/sub-process.c +++ b/sub-process.c @@ -6,12 +6,14 @@ #include "pkt-line.h" int cmd2process_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct subprocess_entry *e1 = entry; - const struct subprocess_entry *e2 = entry_or_key; + const struct subprocess_entry *e1, *e2; + + e1 = container_of(eptr, const struct subprocess_entry, ent); + e2 = container_of(entry_or_key, const struct subprocess_entry, ent); return strcmp(e1->cmd, e2->cmd); } diff --git a/sub-process.h b/sub-process.h index 5c182fad98..0d12708b8c 100644 --- a/sub-process.h +++ b/sub-process.h @@ -43,8 +43,8 @@ struct subprocess_capability { /* Function to test two subprocess hashmap entries for equality. */ int cmd2process_cmp(const void *unused_cmp_data, - const void *e1, - const void *e2, + const struct hashmap_entry *e, + const struct hashmap_entry *entry_or_key, const void *unused_keydata); /* diff --git a/submodule-config.c b/submodule-config.c index b031884789..5463729ab8 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -38,24 +38,28 @@ enum lookup_type { }; static int config_path_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct submodule_entry *a = entry; - const struct submodule_entry *b = entry_or_key; + const struct submodule_entry *a, *b; + + a = container_of(eptr, const struct submodule_entry, ent); + b = container_of(entry_or_key, const struct submodule_entry, ent); return strcmp(a->config->path, b->config->path) || !oideq(&a->config->gitmodules_oid, &b->config->gitmodules_oid); } static int config_name_cmp(const void *unused_cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *unused_keydata) { - const struct submodule_entry *a = entry; - const struct submodule_entry *b = entry_or_key; + const struct submodule_entry *a, *b; + + a = container_of(eptr, const struct submodule_entry, ent); + b = container_of(entry_or_key, const struct submodule_entry, ent); return strcmp(a->config->name, b->config->name) || !oideq(&a->config->gitmodules_oid, &b->config->gitmodules_oid); diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index e82cbfdee2..56846da64c 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -16,15 +16,17 @@ static const char *get_value(const struct test_entry *e) } static int test_entry_cmp(const void *cmp_data, - const void *entry, - const void *entry_or_key, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, const void *keydata) { const int ignore_case = cmp_data ? *((int *)cmp_data) : 0; - const struct test_entry *e1 = entry; - const struct test_entry *e2 = entry_or_key; + const struct test_entry *e1, *e2; const char *key = keydata; + e1 = container_of(eptr, const struct test_entry, ent); + e2 = container_of(entry_or_key, const struct test_entry, ent); + if (ignore_case) return strcasecmp(e1->key, key ? key : e2->key); else From 87571c3f71ba41d89eef5202f8589daa26f984ca Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:38 +0000 Subject: [PATCH 15/20] hashmap: use *_entry APIs for iteration Inspired by list_for_each_entry in the Linux kernel. Once again, these are somewhat compromised usability-wise by compilers lacking __typeof__ support. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 5 +++-- blame.c | 10 ++++++---- builtin/describe.c | 4 ++-- builtin/difftool.c | 8 ++++---- config.c | 5 +++-- hashmap.c | 2 +- hashmap.h | 15 +++++++++++++-- merge-recursive.c | 25 +++++++++++++++---------- oidmap.h | 6 ++++-- revision.c | 10 ++++++---- submodule-config.c | 8 +++++--- t/helper/test-hashmap.c | 7 ++++--- t/helper/test-lazy-init-name-hash.c | 12 ++++-------- 13 files changed, 70 insertions(+), 47 deletions(-) diff --git a/attr.c b/attr.c index 6053481610..ca8be46e8e 100644 --- a/attr.c +++ b/attr.c @@ -163,12 +163,13 @@ static void all_attrs_init(struct attr_hashmap *map, struct attr_check *check) if (size != check->all_attrs_nr) { struct attr_hash_entry *e; struct hashmap_iter iter; - hashmap_iter_init(&map->map, &iter); REALLOC_ARRAY(check->all_attrs, size); check->all_attrs_nr = size; - while ((e = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(&map->map, &iter, e, + struct attr_hash_entry, + ent /* member name */) { const struct git_attr *a = e->value; check->all_attrs[a->attr_nr].attr = a; } diff --git a/blame.c b/blame.c index aa46c7ec52..3d8accf902 100644 --- a/blame.c +++ b/blame.c @@ -450,9 +450,9 @@ static int fingerprint_similarity(struct fingerprint *a, struct fingerprint *b) struct hashmap_iter iter; const struct fingerprint_entry *entry_a, *entry_b; - hashmap_iter_init(&b->map, &iter); - - while ((entry_b = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(&b->map, &iter, entry_b, + const struct fingerprint_entry, + entry /* member name */) { entry_a = hashmap_get_entry(&a->map, entry_b, NULL, struct fingerprint_entry, entry); if (entry_a) { @@ -473,7 +473,9 @@ static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b) hashmap_iter_init(&b->map, &iter); - while ((entry_b = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(&b->map, &iter, entry_b, + const struct fingerprint_entry, + entry /* member name */) { entry_a = hashmap_get_entry(&a->map, entry_b, NULL, struct fingerprint_entry, entry); if (entry_a) { diff --git a/builtin/describe.c b/builtin/describe.c index e9267b5c9c..8cf2cd992d 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -333,8 +333,8 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst) struct commit_name *n; init_commit_names(&commit_names); - n = hashmap_iter_first(&names, &iter); - for (; n; n = hashmap_iter_next(&iter)) { + hashmap_for_each_entry(&names, &iter, n, struct commit_name, + entry /* member name */) { c = lookup_commit_reference_gently(the_repository, &n->peeled, 1); if (c) diff --git a/builtin/difftool.c b/builtin/difftool.c index 4a37b3edee..dd94179b68 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -538,8 +538,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, * temporary file to both the left and right directories to show the * change in the recorded SHA1 for the submodule. */ - hashmap_iter_init(&submodules, &iter); - while ((entry = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(&submodules, &iter, entry, + struct pair_entry, entry /* member name */) { if (*entry->left) { add_path(&ldir, ldir_len, entry->path); ensure_leading_directories(ldir.buf); @@ -557,8 +557,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, * shows only the link itself, not the contents of the link target. * This loop replicates that behavior. */ - hashmap_iter_init(&symlinks2, &iter); - while ((entry = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(&symlinks2, &iter, entry, + struct pair_entry, entry /* member name */) { if (*entry->left) { add_path(&ldir, ldir_len, entry->path); ensure_leading_directories(ldir.buf); diff --git a/config.c b/config.c index 33043ee73c..8433f74371 100644 --- a/config.c +++ b/config.c @@ -1942,8 +1942,9 @@ void git_configset_clear(struct config_set *cs) if (!cs->hash_initialized) return; - hashmap_iter_init(&cs->config_hash, &iter); - while ((entry = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(&cs->config_hash, &iter, entry, + struct config_set_element, + ent /* member name */) { free(entry->key); string_list_clear(&entry->value_list, 1); } diff --git a/hashmap.c b/hashmap.c index 64f6accfff..deb5fdf28c 100644 --- a/hashmap.c +++ b/hashmap.c @@ -256,7 +256,7 @@ void hashmap_iter_init(struct hashmap *map, struct hashmap_iter *iter) iter->next = NULL; } -void *hashmap_iter_next(struct hashmap_iter *iter) +struct hashmap_entry *hashmap_iter_next(struct hashmap_iter *iter) { struct hashmap_entry *current = iter->next; for (;;) { diff --git a/hashmap.h b/hashmap.h index 8f5c163d56..8d4b3907b4 100644 --- a/hashmap.h +++ b/hashmap.h @@ -382,16 +382,27 @@ struct hashmap_iter { void hashmap_iter_init(struct hashmap *map, struct hashmap_iter *iter); /* Returns the next hashmap_entry, or NULL if there are no more entries. */ -void *hashmap_iter_next(struct hashmap_iter *iter); +struct hashmap_entry *hashmap_iter_next(struct hashmap_iter *iter); /* Initializes the iterator and returns the first entry, if any. */ -static inline void *hashmap_iter_first(struct hashmap *map, +static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map, struct hashmap_iter *iter) { hashmap_iter_init(map, iter); return hashmap_iter_next(iter); } +#define hashmap_iter_next_entry(iter, type, member) \ + container_of_or_null(hashmap_iter_next(iter), type, member) + +#define hashmap_iter_first_entry(map, iter, type, member) \ + container_of_or_null(hashmap_iter_first(map, iter), type, member) + +#define hashmap_for_each_entry(map, iter, var, type, member) \ + for (var = hashmap_iter_first_entry(map, iter, type, member); \ + var; \ + var = hashmap_iter_next_entry(iter, type, member)) + /* * returns a @pointer of @type matching @keyvar, or NULL if nothing found. * @keyvar is a pointer of @type diff --git a/merge-recursive.c b/merge-recursive.c index b06e9f7f0b..73c7750448 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2135,8 +2135,9 @@ static void handle_directory_level_conflicts(struct merge_options *opt, struct string_list remove_from_head = STRING_LIST_INIT_NODUP; struct string_list remove_from_merge = STRING_LIST_INIT_NODUP; - hashmap_iter_init(dir_re_head, &iter); - while ((head_ent = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(dir_re_head, &iter, head_ent, + struct dir_rename_entry, + ent /* member name */) { merge_ent = dir_rename_find_entry(dir_re_merge, head_ent->dir); if (merge_ent && !head_ent->non_unique_new_dir && @@ -2160,8 +2161,9 @@ static void handle_directory_level_conflicts(struct merge_options *opt, remove_hashmap_entries(dir_re_head, &remove_from_head); remove_hashmap_entries(dir_re_merge, &remove_from_merge); - hashmap_iter_init(dir_re_merge, &iter); - while ((merge_ent = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(dir_re_merge, &iter, merge_ent, + struct dir_rename_entry, + ent /* member name */) { head_ent = dir_rename_find_entry(dir_re_head, merge_ent->dir); if (tree_has_path(opt->repo, merge, merge_ent->dir)) { /* 2. This wasn't a directory rename after all */ @@ -2265,8 +2267,9 @@ static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs) * we set non_unique_new_dir. Once we've determined the winner (or * that there is no winner), we no longer need possible_new_dirs. */ - hashmap_iter_init(dir_renames, &iter); - while ((entry = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(dir_renames, &iter, entry, + struct dir_rename_entry, + ent /* member name */) { int max = 0; int bad_max = 0; char *best = NULL; @@ -2624,8 +2627,9 @@ static struct string_list *get_renames(struct merge_options *opt, entries); } - hashmap_iter_init(&collisions, &iter); - while ((e = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(&collisions, &iter, e, + struct collision_entry, + ent /* member name */) { free(e->target_file); string_list_clear(&e->source_files, 0); } @@ -2842,8 +2846,9 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs, struct hashmap_iter iter; struct dir_rename_entry *e; - hashmap_iter_init(dir_renames, &iter); - while ((e = hashmap_iter_next(&iter))) { + hashmap_for_each_entry(dir_renames, &iter, e, + struct dir_rename_entry, + ent /* member name */) { free(e->dir); strbuf_release(&e->new_dir); /* possible_new_dirs already cleared in get_directory_renames */ diff --git a/oidmap.h b/oidmap.h index 7a939461ff..c66a83ab1d 100644 --- a/oidmap.h +++ b/oidmap.h @@ -78,14 +78,16 @@ static inline void oidmap_iter_init(struct oidmap *map, struct oidmap_iter *iter static inline void *oidmap_iter_next(struct oidmap_iter *iter) { - return hashmap_iter_next(&iter->h_iter); + /* TODO: this API could be reworked to do compile-time type checks */ + return (void *)hashmap_iter_next(&iter->h_iter); } static inline void *oidmap_iter_first(struct oidmap *map, struct oidmap_iter *iter) { oidmap_iter_init(map, iter); - return oidmap_iter_next(iter); + /* TODO: this API could be reworked to do compile-time type checks */ + return (void *)oidmap_iter_next(iter); } #endif diff --git a/revision.c b/revision.c index f32fbc5e2e..f28cbe5de8 100644 --- a/revision.c +++ b/revision.c @@ -128,9 +128,10 @@ static void paths_and_oids_clear(struct hashmap *map) { struct hashmap_iter iter; struct path_and_oids_entry *entry; - hashmap_iter_init(map, &iter); - while ((entry = (struct path_and_oids_entry *)hashmap_iter_next(&iter))) { + hashmap_for_each_entry(map, &iter, entry, + struct path_and_oids_entry, + ent /* member name */) { oidset_clear(&entry->trees); free(entry->path); } @@ -242,8 +243,9 @@ void mark_trees_uninteresting_sparse(struct repository *r, add_children_by_path(r, tree, &map); } - hashmap_iter_init(&map, &map_iter); - while ((entry = hashmap_iter_next(&map_iter))) + hashmap_for_each_entry(&map, &map_iter, entry, + struct path_and_oids_entry, + ent /* member name */) mark_trees_uninteresting_sparse(r, &entry->trees); paths_and_oids_clear(&map); diff --git a/submodule-config.c b/submodule-config.c index 5463729ab8..5319933e1d 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -99,8 +99,8 @@ static void submodule_cache_clear(struct submodule_cache *cache) * allocation of struct submodule entries. Each is allocated by * their .gitmodules blob sha1 and submodule name. */ - hashmap_iter_init(&cache->for_name, &iter); - while ((entry = hashmap_iter_next(&iter))) + hashmap_for_each_entry(&cache->for_name, &iter, entry, + struct submodule_entry, ent /* member name */) free_one_config(entry); hashmap_free(&cache->for_path, 1); @@ -556,7 +556,9 @@ static const struct submodule *config_from(struct submodule_cache *cache, struct hashmap_iter iter; struct submodule_entry *entry; - entry = hashmap_iter_first(&cache->for_name, &iter); + entry = hashmap_iter_first_entry(&cache->for_name, &iter, + struct submodule_entry, + ent /* member name */); if (!entry) return NULL; return entry->config; diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index 56846da64c..4ec5e11556 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -222,10 +222,11 @@ int cmd__hashmap(int argc, const char **argv) free(entry); } else if (!strcmp("iterate", cmd)) { - struct hashmap_iter iter; - hashmap_iter_init(&map, &iter); - while ((entry = hashmap_iter_next(&iter))) + + hashmap_for_each_entry(&map, &iter, entry, + struct test_entry, + ent /* member name */) printf("%s %s\n", entry->key, get_value(entry)); } else if (!strcmp("size", cmd)) { diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c index b99a37080d..9d4664d6a4 100644 --- a/t/helper/test-lazy-init-name-hash.c +++ b/t/helper/test-lazy-init-name-hash.c @@ -41,17 +41,13 @@ static void dump_run(void) die("non-threaded code path used"); } - dir = hashmap_iter_first(&the_index.dir_hash, &iter_dir); - while (dir) { + hashmap_for_each_entry(&the_index.dir_hash, &iter_dir, dir, + struct dir_entry, ent /* member name */) printf("dir %08x %7d %s\n", dir->ent.hash, dir->nr, dir->name); - dir = hashmap_iter_next(&iter_dir); - } - ce = hashmap_iter_first(&the_index.name_hash, &iter_cache); - while (ce) { + hashmap_for_each_entry(&the_index.name_hash, &iter_cache, ce, + struct cache_entry, ent /* member name */) printf("name %08x %s\n", ce->ent.hash, ce->name); - ce = hashmap_iter_next(&iter_cache); - } discard_cache(); } From 8a973d0bb398d6d83d6c048acecc750d01bd7234 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:39 +0000 Subject: [PATCH 16/20] hashmap: hashmap_{put,remove} return hashmap_entry * And add *_entry variants to perform container_of as necessary to simplify most callers. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- hashmap.c | 8 +++++--- hashmap.h | 15 ++++++++++++--- range-diff.c | 4 +++- remote.c | 3 ++- submodule-config.c | 4 +++- t/helper/test-hashmap.c | 9 +++++++-- 6 files changed, 32 insertions(+), 11 deletions(-) diff --git a/hashmap.c b/hashmap.c index deb5fdf28c..1b60f97cf2 100644 --- a/hashmap.c +++ b/hashmap.c @@ -219,8 +219,9 @@ void hashmap_add(struct hashmap *map, struct hashmap_entry *entry) } } -void *hashmap_remove(struct hashmap *map, const struct hashmap_entry *key, - const void *keydata) +struct hashmap_entry *hashmap_remove(struct hashmap *map, + const struct hashmap_entry *key, + const void *keydata) { struct hashmap_entry *old; struct hashmap_entry **e = find_entry_ptr(map, key, keydata); @@ -242,7 +243,8 @@ void *hashmap_remove(struct hashmap *map, const struct hashmap_entry *key, return old; } -void *hashmap_put(struct hashmap *map, struct hashmap_entry *entry) +struct hashmap_entry *hashmap_put(struct hashmap *map, + struct hashmap_entry *entry) { struct hashmap_entry *old = hashmap_remove(map, entry, NULL); hashmap_add(map, entry); diff --git a/hashmap.h b/hashmap.h index 8d4b3907b4..bc3b10e097 100644 --- a/hashmap.h +++ b/hashmap.h @@ -349,7 +349,11 @@ void hashmap_add(struct hashmap *map, struct hashmap_entry *entry); * `entry` is the entry to add or replace. * Returns the replaced entry, or NULL if not found (i.e. the entry was added). */ -void *hashmap_put(struct hashmap *map, struct hashmap_entry *entry); +struct hashmap_entry *hashmap_put(struct hashmap *map, + struct hashmap_entry *entry); + +#define hashmap_put_entry(map, keyvar, type, member) \ + container_of_or_null(hashmap_put(map, &(keyvar)->member), type, member) /* * Removes a hashmap entry matching the specified key. If the hashmap contains @@ -358,8 +362,13 @@ void *hashmap_put(struct hashmap *map, struct hashmap_entry *entry); * * Argument explanation is the same as in `hashmap_get`. */ -void *hashmap_remove(struct hashmap *map, const struct hashmap_entry *key, - const void *keydata); +struct hashmap_entry *hashmap_remove(struct hashmap *map, + const struct hashmap_entry *key, + const void *keydata); + +#define hashmap_remove_entry(map, keyvar, keydata, type, member) \ + container_of_or_null(hashmap_remove(map, &(keyvar)->member, keydata), \ + type, member) /* * Returns the `bucket` an entry is stored in. diff --git a/range-diff.c b/range-diff.c index c51cfd5556..e5e7820bfe 100644 --- a/range-diff.c +++ b/range-diff.c @@ -229,7 +229,9 @@ static void find_exact_matches(struct string_list *a, struct string_list *b) util->patch = b->items[i].string; util->diff = util->patch + util->diff_offset; hashmap_entry_init(&util->e, strhash(util->diff)); - other = hashmap_remove(&map, &util->e, NULL); + other = hashmap_remove_entry(&map, util, NULL, + struct patch_util, + e /* member name */); if (other) { if (other->matching >= 0) BUG("already assigned!"); diff --git a/remote.c b/remote.c index fa9cadcfbd..5fcddcd88d 100644 --- a/remote.c +++ b/remote.c @@ -162,7 +162,8 @@ static struct remote *make_remote(const char *name, int len) remotes[remotes_nr++] = ret; hashmap_entry_init(&ret->ent, lookup_entry.hash); - replaced = hashmap_put(&remotes_hash, &ret->ent); + replaced = hashmap_put_entry(&remotes_hash, ret, struct remote, + ent /* member name */); assert(replaced == NULL); /* no previous entry overwritten */ return ret; } diff --git a/submodule-config.c b/submodule-config.c index 5319933e1d..a289d195f6 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -141,7 +141,9 @@ static void cache_remove_path(struct submodule_cache *cache, struct submodule_entry *removed; hashmap_entry_init(&e.ent, hash); e.config = submodule; - removed = hashmap_remove(&cache->for_path, &e.ent, NULL); + removed = hashmap_remove_entry(&cache->for_path, &e, NULL, + struct submodule_entry, + ent /* member name */); free(removed); } diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index 4ec5e11556..07a93a2aec 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -189,7 +189,9 @@ int cmd__hashmap(int argc, const char **argv) entry = alloc_test_entry(hash, p1, p2); /* add / replace entry */ - entry = hashmap_put(&map, &entry->ent); + entry = hashmap_put_entry(&map, entry, + struct test_entry, + ent /* member name */); /* print and free replaced entry, if any */ puts(entry ? get_value(entry) : "NULL"); @@ -212,10 +214,13 @@ int cmd__hashmap(int argc, const char **argv) /* setup static key */ struct hashmap_entry key; + struct hashmap_entry *rm; hashmap_entry_init(&key, hash); /* remove entry from hashmap */ - entry = hashmap_remove(&map, &key, p1); + rm = hashmap_remove(&map, &key, p1); + entry = rm ? container_of(rm, struct test_entry, ent) + : NULL; /* print result and free entry*/ puts(entry ? get_value(entry) : "NULL"); From c8e424c9c94d97b18cd335be17f32a8ce94a5b7f Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:40 +0000 Subject: [PATCH 17/20] hashmap: introduce hashmap_free_entries `hashmap_free_entries' behaves like `container_of' and passes the offset of the hashmap_entry struct to the internal `hashmap_free_' function, allowing the function to free any struct pointer regardless of where the hashmap_entry field is located. `hashmap_free' no longer takes any arguments aside from the hashmap itself. Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- blame.c | 2 +- builtin/fetch.c | 6 +++--- config.c | 2 +- diff.c | 6 ++++-- diffcore-rename.c | 2 +- hashmap.c | 11 ++++++++--- hashmap.h | 19 +++++++++++++------ merge-recursive.c | 7 ++++--- name-hash.c | 4 ++-- oidmap.c | 4 +++- patch-ids.c | 2 +- range-diff.c | 2 +- ref-filter.c | 3 ++- revision.c | 2 +- sequencer.c | 4 ++-- submodule-config.c | 4 ++-- t/helper/test-hashmap.c | 6 +++--- 17 files changed, 52 insertions(+), 34 deletions(-) diff --git a/blame.c b/blame.c index 3d8accf902..f33af0da9f 100644 --- a/blame.c +++ b/blame.c @@ -433,7 +433,7 @@ static void get_fingerprint(struct fingerprint *result, static void free_fingerprint(struct fingerprint *f) { - hashmap_free(&f->map, 0); + hashmap_free(&f->map); free(f->entries); } diff --git a/builtin/fetch.c b/builtin/fetch.c index 476c2416e3..09f7170616 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -366,7 +366,7 @@ static void find_non_local_tags(const struct ref *refs, item = refname_hash_add(&remote_refs, ref->name, &ref->old_oid); string_list_insert(&remote_refs_list, ref->name); } - hashmap_free(&existing_refs, 1); + hashmap_free_entries(&existing_refs, struct refname_hash_entry, ent); /* * We may have a final lightweight tag that needs to be @@ -401,7 +401,7 @@ static void find_non_local_tags(const struct ref *refs, **tail = rm; *tail = &rm->next; } - hashmap_free(&remote_refs, 1); + hashmap_free_entries(&remote_refs, struct refname_hash_entry, ent); string_list_clear(&remote_refs_list, 0); } @@ -530,7 +530,7 @@ static struct ref *get_ref_map(struct remote *remote, } } } - hashmap_free(&existing_refs, 1); + hashmap_free_entries(&existing_refs, struct refname_hash_entry, ent); return ref_map; } diff --git a/config.c b/config.c index 8433f74371..4d05dbc15a 100644 --- a/config.c +++ b/config.c @@ -1948,7 +1948,7 @@ void git_configset_clear(struct config_set *cs) free(entry->key); string_list_clear(&entry->value_list, 1); } - hashmap_free(&cs->config_hash, 1); + hashmap_free_entries(&cs->config_hash, struct config_set_element, ent); cs->hash_initialized = 0; free(cs->list.items); cs->list.nr = 0; diff --git a/diff.c b/diff.c index 5eaf689fcc..f94d9f96af 100644 --- a/diff.c +++ b/diff.c @@ -6236,8 +6236,10 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o) if (o->color_moved == COLOR_MOVED_ZEBRA_DIM) dim_moved_lines(o); - hashmap_free(&add_lines, 1); - hashmap_free(&del_lines, 1); + hashmap_free_entries(&add_lines, struct moved_entry, + ent); + hashmap_free_entries(&del_lines, struct moved_entry, + ent); } for (i = 0; i < esm.nr; i++) diff --git a/diffcore-rename.c b/diffcore-rename.c index 611b08f463..994609ed58 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -358,7 +358,7 @@ static int find_exact_renames(struct diff_options *options) renames += find_identical_files(&file_table, i, options); /* Free the hash data structure and entries */ - hashmap_free(&file_table, 1); + hashmap_free_entries(&file_table, struct file_similarity, entry); return renames; } diff --git a/hashmap.c b/hashmap.c index 1b60f97cf2..65b447f6cd 100644 --- a/hashmap.c +++ b/hashmap.c @@ -171,16 +171,21 @@ void hashmap_init(struct hashmap *map, hashmap_cmp_fn equals_function, map->do_count_items = 1; } -void hashmap_free(struct hashmap *map, int free_entries) +void hashmap_free_(struct hashmap *map, ssize_t entry_offset) { if (!map || !map->table) return; - if (free_entries) { + if (entry_offset >= 0) { /* called by hashmap_free_entries */ struct hashmap_iter iter; struct hashmap_entry *e; + hashmap_iter_init(map, &iter); while ((e = hashmap_iter_next(&iter))) - free(e); + /* + * like container_of, but using caller-calculated + * offset (caller being hashmap_free_entries) + */ + free((char *)e - entry_offset); } free(map->table); memset(map, 0, sizeof(*map)); diff --git a/hashmap.h b/hashmap.h index bc3b10e097..171d6ddb76 100644 --- a/hashmap.h +++ b/hashmap.h @@ -96,7 +96,7 @@ * } * * if (!strcmp("end", action)) { - * hashmap_free(&map, 1); + * hashmap_free_entries(&map, struct long2string, ent); * break; * } * } @@ -232,13 +232,20 @@ void hashmap_init(struct hashmap *map, const void *equals_function_data, size_t initial_size); +/* internal function for freeing hashmap */ +void hashmap_free_(struct hashmap *map, ssize_t offset); + /* - * Frees a hashmap structure and allocated memory. - * - * If `free_entries` is true, each hashmap_entry in the map is freed as well - * using stdlibs free(). + * Frees a hashmap structure and allocated memory, leaves entries undisturbed */ -void hashmap_free(struct hashmap *map, int free_entries); +#define hashmap_free(map) hashmap_free_(map, -1) + +/* + * Frees @map and all entries. @type is the struct type of the entry + * where @member is the hashmap_entry struct used to associate with @map + */ +#define hashmap_free_entries(map, type, member) \ + hashmap_free_(map, offsetof(type, member)); /* hashmap_entry functions */ diff --git a/merge-recursive.c b/merge-recursive.c index 73c7750448..34b3d54154 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2633,7 +2633,7 @@ static struct string_list *get_renames(struct merge_options *opt, free(e->target_file); string_list_clear(&e->source_files, 0); } - hashmap_free(&collisions, 1); + hashmap_free_entries(&collisions, struct collision_entry, ent); return renames; } @@ -2853,7 +2853,7 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs, strbuf_release(&e->new_dir); /* possible_new_dirs already cleared in get_directory_renames */ } - hashmap_free(dir_renames, 1); + hashmap_free_entries(dir_renames, struct dir_rename_entry, ent); free(dir_renames); free(pairs->queue); @@ -3482,7 +3482,8 @@ int merge_trees(struct merge_options *opt, string_list_clear(entries, 1); free(entries); - hashmap_free(&opt->current_file_dir_set, 1); + hashmap_free_entries(&opt->current_file_dir_set, + struct path_hashmap_entry, e); if (clean < 0) { unpack_trees_finish(opt); diff --git a/name-hash.c b/name-hash.c index 85a1ce982c..c86fe0f1df 100644 --- a/name-hash.c +++ b/name-hash.c @@ -728,6 +728,6 @@ void free_name_hash(struct index_state *istate) return; istate->name_hash_initialized = 0; - hashmap_free(&istate->name_hash, 0); - hashmap_free(&istate->dir_hash, 1); + hashmap_free(&istate->name_hash); + hashmap_free_entries(&istate->dir_hash, struct dir_entry, ent); } diff --git a/oidmap.c b/oidmap.c index 4942599391..423aa014a3 100644 --- a/oidmap.c +++ b/oidmap.c @@ -25,7 +25,9 @@ void oidmap_free(struct oidmap *map, int free_entries) { if (!map) return; - hashmap_free(&map->map, free_entries); + + /* TODO: make oidmap itself not depend on struct layouts */ + hashmap_free_(&map->map, free_entries ? 0 : -1); } void *oidmap_get(const struct oidmap *map, const struct object_id *key) diff --git a/patch-ids.c b/patch-ids.c index 75f8c9f1a1..af17828e33 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -71,7 +71,7 @@ int init_patch_ids(struct repository *r, struct patch_ids *ids) int free_patch_ids(struct patch_ids *ids) { - hashmap_free(&ids->patches, 1); + hashmap_free_entries(&ids->patches, struct patch_id, ent); return 0; } diff --git a/range-diff.c b/range-diff.c index e5e7820bfe..9df53569bb 100644 --- a/range-diff.c +++ b/range-diff.c @@ -241,7 +241,7 @@ static void find_exact_matches(struct string_list *a, struct string_list *b) } } - hashmap_free(&map, 0); + hashmap_free(&map); } static void diffsize_consume(void *data, char *line, unsigned long len) diff --git a/ref-filter.c b/ref-filter.c index 4613df8826..0950b789e3 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2172,7 +2172,8 @@ void ref_array_clear(struct ref_array *array) used_atom_cnt = 0; if (ref_to_worktree_map.worktrees) { - hashmap_free(&(ref_to_worktree_map.map), 1); + hashmap_free_entries(&(ref_to_worktree_map.map), + struct ref_to_worktree_entry, ent); free_worktrees(ref_to_worktree_map.worktrees); ref_to_worktree_map.worktrees = NULL; } diff --git a/revision.c b/revision.c index f28cbe5de8..8a5f866ae6 100644 --- a/revision.c +++ b/revision.c @@ -136,7 +136,7 @@ static void paths_and_oids_clear(struct hashmap *map) free(entry->path); } - hashmap_free(map, 1); + hashmap_free_entries(map, struct path_and_oids_entry, ent); } static void paths_and_oids_insert(struct hashmap *map, diff --git a/sequencer.c b/sequencer.c index b3e7319b55..694b463518 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4772,7 +4772,7 @@ static int make_script_with_merges(struct pretty_print_context *pp, oidmap_free(&commit2todo, 1); oidmap_free(&state.commit2label, 1); - hashmap_free(&state.labels, 1); + hashmap_free_entries(&state.labels, struct labels_entry, entry); strbuf_release(&state.buf); return 0; @@ -5301,7 +5301,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list) for (i = 0; i < todo_list->nr; i++) free(subjects[i]); free(subjects); - hashmap_free(&subject2item, 1); + hashmap_free_entries(&subject2item, struct subject2item_entry, entry); clear_commit_todo_item(&commit_todo); diff --git a/submodule-config.c b/submodule-config.c index a289d195f6..5462acc8ec 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -103,8 +103,8 @@ static void submodule_cache_clear(struct submodule_cache *cache) struct submodule_entry, ent /* member name */) free_one_config(entry); - hashmap_free(&cache->for_path, 1); - hashmap_free(&cache->for_name, 1); + hashmap_free_entries(&cache->for_path, struct submodule_entry, ent); + hashmap_free_entries(&cache->for_name, struct submodule_entry, ent); cache->initialized = 0; cache->gitmodules_read = 0; } diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index 07a93a2aec..6f2530dcc8 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -109,7 +109,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds) hashmap_add(&map, &entries[i]->ent); } - hashmap_free(&map, 0); + hashmap_free(&map); } } else { /* test map lookups */ @@ -129,7 +129,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds) } } - hashmap_free(&map, 0); + hashmap_free(&map); } } @@ -266,6 +266,6 @@ int cmd__hashmap(int argc, const char **argv) } strbuf_release(&line); - hashmap_free(&map, 1); + hashmap_free_entries(&map, struct test_entry, ent); return 0; } From 23dee69f53cf5024ca79e0b707dcb03c63f33bef Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:41 +0000 Subject: [PATCH 18/20] OFFSETOF_VAR macro to simplify hashmap iterators While we cannot rely on a `__typeof__' operator being portable to use with `offsetof'; we can calculate the pointer offset using an existing pointer and the address of a member using pointer arithmetic for compilers without `__typeof__'. This allows us to simplify usage of hashmap iterator macros by not having to specify a type when a pointer of that type is already given. In the future, list iterator macros (e.g. list_for_each_entry) may also be implemented using OFFSETOF_VAR to save hackers the trouble of using container_of/list_entry macros and without relying on non-portable `__typeof__'. v3: use `__typeof__' to avoid clang warnings Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 1 - blame.c | 2 -- builtin/describe.c | 2 +- builtin/difftool.c | 4 +-- config.c | 1 - diff.c | 5 ++-- diffcore-rename.c | 2 +- git-compat-util.h | 13 +++++++++ hashmap.h | 44 ++++++++++++++++++++--------- merge-recursive.c | 5 ---- name-hash.c | 3 +- revision.c | 8 ++---- submodule-config.c | 2 +- t/helper/test-hashmap.c | 5 +--- t/helper/test-lazy-init-name-hash.c | 4 +-- 15 files changed, 56 insertions(+), 45 deletions(-) diff --git a/attr.c b/attr.c index ca8be46e8e..9849106627 100644 --- a/attr.c +++ b/attr.c @@ -168,7 +168,6 @@ static void all_attrs_init(struct attr_hashmap *map, struct attr_check *check) check->all_attrs_nr = size; hashmap_for_each_entry(&map->map, &iter, e, - struct attr_hash_entry, ent /* member name */) { const struct git_attr *a = e->value; check->all_attrs[a->attr_nr].attr = a; diff --git a/blame.c b/blame.c index f33af0da9f..90b247abf9 100644 --- a/blame.c +++ b/blame.c @@ -451,7 +451,6 @@ static int fingerprint_similarity(struct fingerprint *a, struct fingerprint *b) const struct fingerprint_entry *entry_a, *entry_b; hashmap_for_each_entry(&b->map, &iter, entry_b, - const struct fingerprint_entry, entry /* member name */) { entry_a = hashmap_get_entry(&a->map, entry_b, NULL, struct fingerprint_entry, entry); @@ -474,7 +473,6 @@ static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b) hashmap_iter_init(&b->map, &iter); hashmap_for_each_entry(&b->map, &iter, entry_b, - const struct fingerprint_entry, entry /* member name */) { entry_a = hashmap_get_entry(&a->map, entry_b, NULL, struct fingerprint_entry, entry); diff --git a/builtin/describe.c b/builtin/describe.c index 8cf2cd992d..1caf98f716 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -333,7 +333,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst) struct commit_name *n; init_commit_names(&commit_names); - hashmap_for_each_entry(&names, &iter, n, struct commit_name, + hashmap_for_each_entry(&names, &iter, n, entry /* member name */) { c = lookup_commit_reference_gently(the_repository, &n->peeled, 1); diff --git a/builtin/difftool.c b/builtin/difftool.c index dd94179b68..f2d4d1e0f8 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -539,7 +539,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, * change in the recorded SHA1 for the submodule. */ hashmap_for_each_entry(&submodules, &iter, entry, - struct pair_entry, entry /* member name */) { + entry /* member name */) { if (*entry->left) { add_path(&ldir, ldir_len, entry->path); ensure_leading_directories(ldir.buf); @@ -558,7 +558,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, * This loop replicates that behavior. */ hashmap_for_each_entry(&symlinks2, &iter, entry, - struct pair_entry, entry /* member name */) { + entry /* member name */) { if (*entry->left) { add_path(&ldir, ldir_len, entry->path); ensure_leading_directories(ldir.buf); diff --git a/config.c b/config.c index 4d05dbc15a..77ed00bfbf 100644 --- a/config.c +++ b/config.c @@ -1943,7 +1943,6 @@ void git_configset_clear(struct config_set *cs) return; hashmap_for_each_entry(&cs->config_hash, &iter, entry, - struct config_set_element, ent /* member name */) { free(entry->key); string_list_clear(&entry->value_list, 1); diff --git a/diff.c b/diff.c index f94d9f96af..051de9832d 100644 --- a/diff.c +++ b/diff.c @@ -1038,7 +1038,7 @@ static void pmb_advance_or_null_multi_match(struct diff_options *o, int i; char *got_match = xcalloc(1, pmb_nr); - hashmap_for_each_entry_from(hm, match, struct moved_entry, ent) { + hashmap_for_each_entry_from(hm, match, ent) { for (i = 0; i < pmb_nr; i++) { struct moved_entry *prev = pmb[i].match; struct moved_entry *cur = (prev && prev->next_line) ? @@ -1193,8 +1193,7 @@ static void mark_color_as_moved(struct diff_options *o, * The current line is the start of a new block. * Setup the set of potential blocks. */ - hashmap_for_each_entry_from(hm, match, - struct moved_entry, ent) { + hashmap_for_each_entry_from(hm, match, ent) { ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc); if (o->color_moved_ws_handling & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) { diff --git a/diffcore-rename.c b/diffcore-rename.c index 994609ed58..9ad4dc395a 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -284,7 +284,7 @@ static int find_identical_files(struct hashmap *srcs, */ p = hashmap_get_entry_from_hash(srcs, hash, NULL, struct file_similarity, entry); - hashmap_for_each_entry_from(srcs, p, struct file_similarity, entry) { + hashmap_for_each_entry_from(srcs, p, entry) { int score; struct diff_filespec *source = p->filespec; diff --git a/git-compat-util.h b/git-compat-util.h index 4a23b9090b..8605cb4202 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1337,4 +1337,17 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset) #define container_of_or_null(ptr, type, member) \ (type *)container_of_or_null_offset(ptr, offsetof(type, member)) +/* + * like offsetof(), but takes a pointer to a a variable of type which + * contains @member, instead of a specified type. + * @ptr is subject to multiple evaluation since we can't rely on __typeof__ + * everywhere. + */ +#if defined(__GNUC__) /* clang sets this, too */ +#define OFFSETOF_VAR(ptr, member) offsetof(__typeof__(*ptr), member) +#else /* !__GNUC__ */ +#define OFFSETOF_VAR(ptr, member) \ + ((uintptr_t)&(ptr)->member - (uintptr_t)(ptr)) +#endif /* !__GNUC__ */ + #endif diff --git a/hashmap.h b/hashmap.h index 171d6ddb76..96786c724a 100644 --- a/hashmap.h +++ b/hashmap.h @@ -408,16 +408,32 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map, return hashmap_iter_next(iter); } -#define hashmap_iter_next_entry(iter, type, member) \ - container_of_or_null(hashmap_iter_next(iter), type, member) - +/* + * returns the first entry in @map using @iter, where the entry is of + * @type (e.g. "struct foo") and @member is the name of the + * "struct hashmap_entry" in @type + */ #define hashmap_iter_first_entry(map, iter, type, member) \ container_of_or_null(hashmap_iter_first(map, iter), type, member) -#define hashmap_for_each_entry(map, iter, var, type, member) \ - for (var = hashmap_iter_first_entry(map, iter, type, member); \ +/* internal macro for hashmap_for_each_entry */ +#define hashmap_iter_next_entry_offset(iter, offset) \ + container_of_or_null_offset(hashmap_iter_next(iter), offset) + +/* internal macro for hashmap_for_each_entry */ +#define hashmap_iter_first_entry_offset(map, iter, offset) \ + container_of_or_null_offset(hashmap_iter_first(map, iter), offset) + +/* + * iterate through @map using @iter, @var is a pointer to a type + * containing a @member which is a "struct hashmap_entry" + */ +#define hashmap_for_each_entry(map, iter, var, member) \ + for (var = hashmap_iter_first_entry_offset(map, iter, \ + OFFSETOF_VAR(var, member)); \ var; \ - var = hashmap_iter_next_entry(iter, type, member)) + var = hashmap_iter_next_entry_offset(iter, \ + OFFSETOF_VAR(var, member))) /* * returns a @pointer of @type matching @keyvar, or NULL if nothing found. @@ -432,22 +448,22 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map, container_of_or_null(hashmap_get_from_hash(map, hash, keydata), \ type, member) /* - * returns the next equal @type pointer to @var, or NULL if not found. - * @var is a pointer of @type - * @member is the name of the "struct hashmap_entry" field in @type + * returns the next equal pointer to @var, or NULL if not found. + * @var is a pointer of any type containing "struct hashmap_entry" + * @member is the name of the "struct hashmap_entry" field */ -#define hashmap_get_next_entry(map, var, type, member) \ - container_of_or_null(hashmap_get_next(map, &(var)->member), \ - type, member) +#define hashmap_get_next_entry(map, var, member) \ + container_of_or_null_offset(hashmap_get_next(map, &(var)->member), \ + OFFSETOF_VAR(var, member)) /* * iterate @map starting from @var, where @var is a pointer of @type * and @member is the name of the "struct hashmap_entry" field in @type */ -#define hashmap_for_each_entry_from(map, var, type, member) \ +#define hashmap_for_each_entry_from(map, var, member) \ for (; \ var; \ - var = hashmap_get_next_entry(map, var, type, member)) + var = hashmap_get_next_entry(map, var, member)) /* * Disable item counting and automatic rehashing when adding/removing items. diff --git a/merge-recursive.c b/merge-recursive.c index 34b3d54154..3abba3a618 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2136,7 +2136,6 @@ static void handle_directory_level_conflicts(struct merge_options *opt, struct string_list remove_from_merge = STRING_LIST_INIT_NODUP; hashmap_for_each_entry(dir_re_head, &iter, head_ent, - struct dir_rename_entry, ent /* member name */) { merge_ent = dir_rename_find_entry(dir_re_merge, head_ent->dir); if (merge_ent && @@ -2162,7 +2161,6 @@ static void handle_directory_level_conflicts(struct merge_options *opt, remove_hashmap_entries(dir_re_merge, &remove_from_merge); hashmap_for_each_entry(dir_re_merge, &iter, merge_ent, - struct dir_rename_entry, ent /* member name */) { head_ent = dir_rename_find_entry(dir_re_head, merge_ent->dir); if (tree_has_path(opt->repo, merge, merge_ent->dir)) { @@ -2268,7 +2266,6 @@ static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs) * that there is no winner), we no longer need possible_new_dirs. */ hashmap_for_each_entry(dir_renames, &iter, entry, - struct dir_rename_entry, ent /* member name */) { int max = 0; int bad_max = 0; @@ -2628,7 +2625,6 @@ static struct string_list *get_renames(struct merge_options *opt, } hashmap_for_each_entry(&collisions, &iter, e, - struct collision_entry, ent /* member name */) { free(e->target_file); string_list_clear(&e->source_files, 0); @@ -2847,7 +2843,6 @@ static void initial_cleanup_rename(struct diff_queue_struct *pairs, struct dir_rename_entry *e; hashmap_for_each_entry(dir_renames, &iter, e, - struct dir_rename_entry, ent /* member name */) { free(e->dir); strbuf_release(&e->new_dir); diff --git a/name-hash.c b/name-hash.c index c86fe0f1df..3cda22b657 100644 --- a/name-hash.c +++ b/name-hash.c @@ -714,8 +714,7 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na ce = hashmap_get_entry_from_hash(&istate->name_hash, hash, NULL, struct cache_entry, ent); - hashmap_for_each_entry_from(&istate->name_hash, ce, - struct cache_entry, ent) { + hashmap_for_each_entry_from(&istate->name_hash, ce, ent) { if (same_name(ce, name, namelen, icase)) return ce; } diff --git a/revision.c b/revision.c index 8a5f866ae6..5abd4a1fe7 100644 --- a/revision.c +++ b/revision.c @@ -129,9 +129,7 @@ static void paths_and_oids_clear(struct hashmap *map) struct hashmap_iter iter; struct path_and_oids_entry *entry; - hashmap_for_each_entry(map, &iter, entry, - struct path_and_oids_entry, - ent /* member name */) { + hashmap_for_each_entry(map, &iter, entry, ent /* member name */) { oidset_clear(&entry->trees); free(entry->path); } @@ -243,9 +241,7 @@ void mark_trees_uninteresting_sparse(struct repository *r, add_children_by_path(r, tree, &map); } - hashmap_for_each_entry(&map, &map_iter, entry, - struct path_and_oids_entry, - ent /* member name */) + hashmap_for_each_entry(&map, &map_iter, entry, ent /* member name */) mark_trees_uninteresting_sparse(r, &entry->trees); paths_and_oids_clear(&map); diff --git a/submodule-config.c b/submodule-config.c index 5462acc8ec..c22855cd38 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -100,7 +100,7 @@ static void submodule_cache_clear(struct submodule_cache *cache) * their .gitmodules blob sha1 and submodule name. */ hashmap_for_each_entry(&cache->for_name, &iter, entry, - struct submodule_entry, ent /* member name */) + ent /* member name */) free_one_config(entry); hashmap_free_entries(&cache->for_path, struct submodule_entry, ent); diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index 6f2530dcc8..f89d1194ef 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -205,10 +205,8 @@ int cmd__hashmap(int argc, const char **argv) /* print result */ if (!entry) puts("NULL"); - hashmap_for_each_entry_from(&map, entry, - struct test_entry, ent) { + hashmap_for_each_entry_from(&map, entry, ent) puts(get_value(entry)); - } } else if (!strcmp("remove", cmd) && p1) { @@ -230,7 +228,6 @@ int cmd__hashmap(int argc, const char **argv) struct hashmap_iter iter; hashmap_for_each_entry(&map, &iter, entry, - struct test_entry, ent /* member name */) printf("%s %s\n", entry->key, get_value(entry)); diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c index 9d4664d6a4..cd1b4c9736 100644 --- a/t/helper/test-lazy-init-name-hash.c +++ b/t/helper/test-lazy-init-name-hash.c @@ -42,11 +42,11 @@ static void dump_run(void) } hashmap_for_each_entry(&the_index.dir_hash, &iter_dir, dir, - struct dir_entry, ent /* member name */) + ent /* member name */) printf("dir %08x %7d %s\n", dir->ent.hash, dir->nr, dir->name); hashmap_for_each_entry(&the_index.name_hash, &iter_cache, ce, - struct cache_entry, ent /* member name */) + ent /* member name */) printf("name %08x %s\n", ce->ent.hash, ce->name); discard_cache(); From 404ab78e39fc74c4eb604b6003642ed264f687a6 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:42 +0000 Subject: [PATCH 19/20] hashmap: remove type arg from hashmap_{get,put,remove}_entry Since these macros already take a `keyvar' pointer of a known type, we can rely on OFFSETOF_VAR to get the correct offset without relying on non-portable `__typeof__' and `offsetof'. Argument order is also rearranged, so `keyvar' and `member' are sequential as they are used as: `keyvar->member' Signed-off-by: Eric Wong Reviewed-by: Derrick Stolee Signed-off-by: Junio C Hamano --- attr.c | 2 +- blame.c | 10 ++++----- builtin/difftool.c | 2 +- builtin/fast-export.c | 2 +- config.c | 3 +-- diff.c | 6 ++---- hashmap.c | 2 +- hashmap.h | 45 ++++++++++++++++++++++++++++++----------- merge-recursive.c | 6 ++---- name-hash.c | 3 +-- patch-ids.c | 3 +-- range-diff.c | 4 +--- remote.c | 3 +-- revision.c | 3 +-- sub-process.c | 3 +-- submodule-config.c | 10 +++------ t/helper/test-hashmap.c | 4 +--- 17 files changed, 56 insertions(+), 55 deletions(-) diff --git a/attr.c b/attr.c index 9849106627..15f0efdf60 100644 --- a/attr.c +++ b/attr.c @@ -103,7 +103,7 @@ static void *attr_hashmap_get(struct attr_hashmap *map, hashmap_entry_init(&k.ent, memhash(key, keylen)); k.key = key; k.keylen = keylen; - e = hashmap_get_entry(&map->map, &k, NULL, struct attr_hash_entry, ent); + e = hashmap_get_entry(&map->map, &k, ent, NULL); return e ? e->value : NULL; } diff --git a/blame.c b/blame.c index 90b247abf9..6384f48133 100644 --- a/blame.c +++ b/blame.c @@ -419,8 +419,8 @@ static void get_fingerprint(struct fingerprint *result, continue; hashmap_entry_init(&entry->entry, hash); - found_entry = hashmap_get_entry(&result->map, entry, NULL, - struct fingerprint_entry, entry); + found_entry = hashmap_get_entry(&result->map, entry, + /* member name */ entry, NULL); if (found_entry) { found_entry->count += 1; } else { @@ -452,8 +452,7 @@ static int fingerprint_similarity(struct fingerprint *a, struct fingerprint *b) hashmap_for_each_entry(&b->map, &iter, entry_b, entry /* member name */) { - entry_a = hashmap_get_entry(&a->map, entry_b, NULL, - struct fingerprint_entry, entry); + entry_a = hashmap_get_entry(&a->map, entry_b, entry, NULL); if (entry_a) { intersection += entry_a->count < entry_b->count ? entry_a->count : entry_b->count; @@ -474,8 +473,7 @@ static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b) hashmap_for_each_entry(&b->map, &iter, entry_b, entry /* member name */) { - entry_a = hashmap_get_entry(&a->map, entry_b, NULL, - struct fingerprint_entry, entry); + entry_a = hashmap_get_entry(&a->map, entry_b, entry, NULL); if (entry_a) { if (entry_a->count <= entry_b->count) hashmap_remove(&a->map, &entry_b->entry, NULL); diff --git a/builtin/difftool.c b/builtin/difftool.c index f2d4d1e0f8..c280e682b2 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -167,7 +167,7 @@ static void add_left_or_right(struct hashmap *map, const char *path, FLEX_ALLOC_STR(e, path, path); hashmap_entry_init(&e->entry, strhash(path)); - existing = hashmap_get_entry(map, e, NULL, struct pair_entry, entry); + existing = hashmap_get_entry(map, e, entry, NULL); if (existing) { free(e); e = existing; diff --git a/builtin/fast-export.c b/builtin/fast-export.c index ef0578bf90..e3de403efd 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -156,7 +156,7 @@ static const void *anonymize_mem(struct hashmap *map, hashmap_entry_init(&key.hash, memhash(orig, *len)); key.orig = orig; key.orig_len = *len; - ret = hashmap_get_entry(map, &key, NULL, struct anonymized_entry, hash); + ret = hashmap_get_entry(map, &key, hash, NULL); if (!ret) { ret = xmalloc(sizeof(*ret)); diff --git a/config.c b/config.c index 77ed00bfbf..a4fa464ed2 100644 --- a/config.c +++ b/config.c @@ -1863,8 +1863,7 @@ static struct config_set_element *configset_find_element(struct config_set *cs, hashmap_entry_init(&k.ent, strhash(normalized_key)); k.key = normalized_key; - found_entry = hashmap_get_entry(&cs->config_hash, &k, NULL, - struct config_set_element, ent); + found_entry = hashmap_get_entry(&cs->config_hash, &k, ent, NULL); free(normalized_key); return found_entry; } diff --git a/diff.c b/diff.c index 051de9832d..a9ecb93af3 100644 --- a/diff.c +++ b/diff.c @@ -1146,15 +1146,13 @@ static void mark_color_as_moved(struct diff_options *o, case DIFF_SYMBOL_PLUS: hm = del_lines; key = prepare_entry(o, n); - match = hashmap_get_entry(hm, key, NULL, - struct moved_entry, ent); + match = hashmap_get_entry(hm, key, ent, NULL); free(key); break; case DIFF_SYMBOL_MINUS: hm = add_lines; key = prepare_entry(o, n); - match = hashmap_get_entry(hm, key, NULL, - struct moved_entry, ent); + match = hashmap_get_entry(hm, key, ent, NULL); free(key); break; default: diff --git a/hashmap.c b/hashmap.c index 65b447f6cd..39c13110bc 100644 --- a/hashmap.c +++ b/hashmap.c @@ -311,7 +311,7 @@ const void *memintern(const void *data, size_t len) /* lookup interned string in pool */ hashmap_entry_init(&key.ent, memhash(data, len)); key.len = len; - e = hashmap_get_entry(&map, &key, data, struct pool_entry, ent); + e = hashmap_get_entry(&map, &key, ent, data); if (!e) { /* not found: create it */ FLEX_ALLOC_MEM(e, data, data, len); diff --git a/hashmap.h b/hashmap.h index 96786c724a..bf4a05937d 100644 --- a/hashmap.h +++ b/hashmap.h @@ -63,7 +63,7 @@ * k.key = key; * * flags &= ~COMPARE_VALUE; - * e = hashmap_get_entry(&map, &k, NULL, struct long2string, ent); + * e = hashmap_get_entry(&map, &k, ent, NULL); * if (e) { * printf("first: %ld %s\n", e->key, e->value); * while ((e = hashmap_get_next_entry(&map, e, @@ -359,8 +359,17 @@ void hashmap_add(struct hashmap *map, struct hashmap_entry *entry); struct hashmap_entry *hashmap_put(struct hashmap *map, struct hashmap_entry *entry); -#define hashmap_put_entry(map, keyvar, type, member) \ - container_of_or_null(hashmap_put(map, &(keyvar)->member), type, member) +/* + * Adds or replaces a hashmap entry contained within @keyvar, + * where @keyvar is a pointer to a struct containing a + * "struct hashmap_entry" @member. + * + * Returns the replaced pointer which is of the same type as @keyvar, + * or NULL if not found. + */ +#define hashmap_put_entry(map, keyvar, member) \ + container_of_or_null_offset(hashmap_put(map, &(keyvar)->member), \ + OFFSETOF_VAR(keyvar, member)) /* * Removes a hashmap entry matching the specified key. If the hashmap contains @@ -373,9 +382,20 @@ struct hashmap_entry *hashmap_remove(struct hashmap *map, const struct hashmap_entry *key, const void *keydata); -#define hashmap_remove_entry(map, keyvar, keydata, type, member) \ - container_of_or_null(hashmap_remove(map, &(keyvar)->member, keydata), \ - type, member) +/* + * Removes a hashmap entry contained within @keyvar, + * where @keyvar is a pointer to a struct containing a + * "struct hashmap_entry" @member. + * + * See `hashmap_get` for an explanation of @keydata + * + * Returns the replaced pointer which is of the same type as @keyvar, + * or NULL if not found. + */ +#define hashmap_remove_entry(map, keyvar, member, keydata) \ + container_of_or_null_offset( \ + hashmap_remove(map, &(keyvar)->member, keydata), \ + OFFSETOF_VAR(keyvar, member)) /* * Returns the `bucket` an entry is stored in. @@ -436,13 +456,14 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map, OFFSETOF_VAR(var, member))) /* - * returns a @pointer of @type matching @keyvar, or NULL if nothing found. - * @keyvar is a pointer of @type - * @member is the name of the "struct hashmap_entry" field in @type + * returns a pointer of type matching @keyvar, or NULL if nothing found. + * @keyvar is a pointer to a struct containing a + * "struct hashmap_entry" @member. */ -#define hashmap_get_entry(map, keyvar, keydata, type, member) \ - container_of_or_null(hashmap_get(map, &(keyvar)->member, keydata), \ - type, member) +#define hashmap_get_entry(map, keyvar, member, keydata) \ + container_of_or_null_offset( \ + hashmap_get(map, &(keyvar)->member, keydata), \ + OFFSETOF_VAR(keyvar, member)) #define hashmap_get_entry_from_hash(map, hash, keydata, type, member) \ container_of_or_null(hashmap_get_from_hash(map, hash, keydata), \ diff --git a/merge-recursive.c b/merge-recursive.c index 3abba3a618..8787a40b0c 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -65,8 +65,7 @@ static struct dir_rename_entry *dir_rename_find_entry(struct hashmap *hashmap, return NULL; hashmap_entry_init(&key.ent, strhash(dir)); key.dir = dir; - return hashmap_get_entry(hashmap, &key, NULL, - struct dir_rename_entry, ent); + return hashmap_get_entry(hashmap, &key, ent, NULL); } static int dir_rename_cmp(const void *unused_cmp_data, @@ -104,8 +103,7 @@ static struct collision_entry *collision_find_entry(struct hashmap *hashmap, hashmap_entry_init(&key.ent, strhash(target_file)); key.target_file = target_file; - return hashmap_get_entry(hashmap, &key, NULL, - struct collision_entry, ent); + return hashmap_get_entry(hashmap, &key, ent, NULL); } static int collision_cmp(const void *unused_cmp_data, diff --git a/name-hash.c b/name-hash.c index 3cda22b657..ceb1d7bd6f 100644 --- a/name-hash.c +++ b/name-hash.c @@ -37,8 +37,7 @@ static struct dir_entry *find_dir_entry__hash(struct index_state *istate, struct dir_entry key; hashmap_entry_init(&key.ent, hash); key.namelen = namelen; - return hashmap_get_entry(&istate->dir_hash, &key, name, - struct dir_entry, ent); + return hashmap_get_entry(&istate->dir_hash, &key, ent, name); } static struct dir_entry *find_dir_entry(struct index_state *istate, diff --git a/patch-ids.c b/patch-ids.c index af17828e33..12aa6d494b 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -101,8 +101,7 @@ struct patch_id *has_commit_patch_id(struct commit *commit, if (init_patch_id_entry(&patch, commit, ids)) return NULL; - return hashmap_get_entry(&ids->patches, &patch, NULL, - struct patch_id, ent); + return hashmap_get_entry(&ids->patches, &patch, ent, NULL); } struct patch_id *add_commit_patch_id(struct commit *commit, diff --git a/range-diff.c b/range-diff.c index 9df53569bb..22ad959cee 100644 --- a/range-diff.c +++ b/range-diff.c @@ -229,9 +229,7 @@ static void find_exact_matches(struct string_list *a, struct string_list *b) util->patch = b->items[i].string; util->diff = util->patch + util->diff_offset; hashmap_entry_init(&util->e, strhash(util->diff)); - other = hashmap_remove_entry(&map, util, NULL, - struct patch_util, - e /* member name */); + other = hashmap_remove_entry(&map, util, e, NULL); if (other) { if (other->matching >= 0) BUG("already assigned!"); diff --git a/remote.c b/remote.c index 5fcddcd88d..5c4666b53a 100644 --- a/remote.c +++ b/remote.c @@ -162,8 +162,7 @@ static struct remote *make_remote(const char *name, int len) remotes[remotes_nr++] = ret; hashmap_entry_init(&ret->ent, lookup_entry.hash); - replaced = hashmap_put_entry(&remotes_hash, ret, struct remote, - ent /* member name */); + replaced = hashmap_put_entry(&remotes_hash, ret, ent); assert(replaced == NULL); /* no previous entry overwritten */ return ret; } diff --git a/revision.c b/revision.c index 5abd4a1fe7..6688f89d0d 100644 --- a/revision.c +++ b/revision.c @@ -151,8 +151,7 @@ static void paths_and_oids_insert(struct hashmap *map, key.path = (char *)path; oidset_init(&key.trees, 0); - entry = hashmap_get_entry(map, &key, NULL, - struct path_and_oids_entry, ent); + entry = hashmap_get_entry(map, &key, ent, NULL); if (!entry) { entry = xcalloc(1, sizeof(struct path_and_oids_entry)); hashmap_entry_init(&entry->ent, hash); diff --git a/sub-process.c b/sub-process.c index ad94f72665..1b1af9dcbd 100644 --- a/sub-process.c +++ b/sub-process.c @@ -24,8 +24,7 @@ struct subprocess_entry *subprocess_find_entry(struct hashmap *hashmap, const ch hashmap_entry_init(&key.ent, strhash(cmd)); key.cmd = cmd; - return hashmap_get_entry(hashmap, &key, NULL, - struct subprocess_entry, ent); + return hashmap_get_entry(hashmap, &key, ent, NULL); } int subprocess_read_status(int fd, struct strbuf *status) diff --git a/submodule-config.c b/submodule-config.c index c22855cd38..401a9b2382 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -141,9 +141,7 @@ static void cache_remove_path(struct submodule_cache *cache, struct submodule_entry *removed; hashmap_entry_init(&e.ent, hash); e.config = submodule; - removed = hashmap_remove_entry(&cache->for_path, &e, NULL, - struct submodule_entry, - ent /* member name */); + removed = hashmap_remove_entry(&cache->for_path, &e, ent, NULL); free(removed); } @@ -172,8 +170,7 @@ static const struct submodule *cache_lookup_path(struct submodule_cache *cache, hashmap_entry_init(&key.ent, hash); key.config = &key_config; - entry = hashmap_get_entry(&cache->for_path, &key, NULL, - struct submodule_entry, ent); + entry = hashmap_get_entry(&cache->for_path, &key, ent, NULL); if (entry) return entry->config; return NULL; @@ -193,8 +190,7 @@ static struct submodule *cache_lookup_name(struct submodule_cache *cache, hashmap_entry_init(&key.ent, hash); key.config = &key_config; - entry = hashmap_get_entry(&cache->for_name, &key, NULL, - struct submodule_entry, ent); + entry = hashmap_get_entry(&cache->for_name, &key, ent, NULL); if (entry) return entry->config; return NULL; diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index f89d1194ef..cc577c8956 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -189,9 +189,7 @@ int cmd__hashmap(int argc, const char **argv) entry = alloc_test_entry(hash, p1, p2); /* add / replace entry */ - entry = hashmap_put_entry(&map, entry, - struct test_entry, - ent /* member name */); + entry = hashmap_put_entry(&map, entry, ent); /* print and free replaced entry, if any */ puts(entry ? get_value(entry) : "NULL"); From e2b5038d8793a1d1f92b62dab82acc0d6b7dbcb7 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 6 Oct 2019 23:30:43 +0000 Subject: [PATCH 20/20] hashmap_entry: remove first member requirement from docs Comments stating that "struct hashmap_entry" must be the first member in a struct are no longer valid. Suggested-by: Phillip Wood Signed-off-by: Eric Wong Signed-off-by: Junio C Hamano --- attr.c | 2 +- builtin/fetch.c | 2 +- hashmap.h | 4 ++-- merge-recursive.h | 4 ++-- ref-filter.c | 2 +- refs.c | 2 +- remote.h | 2 +- sub-process.h | 2 +- t/helper/test-hashmap.c | 1 + 9 files changed, 11 insertions(+), 10 deletions(-) diff --git a/attr.c b/attr.c index 15f0efdf60..e5c951db69 100644 --- a/attr.c +++ b/attr.c @@ -62,7 +62,7 @@ static struct attr_hashmap g_attr_hashmap; /* The container for objects stored in "struct attr_hashmap" */ struct attr_hash_entry { - struct hashmap_entry ent; /* must be the first member! */ + struct hashmap_entry ent; const char *key; /* the key; memory should be owned by value */ size_t keylen; /* length of the key */ void *value; /* the stored value */ diff --git a/builtin/fetch.c b/builtin/fetch.c index 09f7170616..a3154a4edb 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -251,7 +251,7 @@ static int will_fetch(struct ref **head, const unsigned char *sha1) } struct refname_hash_entry { - struct hashmap_entry ent; /* must be the first member */ + struct hashmap_entry ent; struct object_id oid; int ignore; char refname[FLEX_ARRAY]; diff --git a/hashmap.h b/hashmap.h index bf4a05937d..bd2701549f 100644 --- a/hashmap.h +++ b/hashmap.h @@ -13,7 +13,7 @@ * * struct hashmap map; * struct long2string { - * struct hashmap_entry ent; // must be the first member! + * struct hashmap_entry ent; * long key; * char value[FLEX_ARRAY]; // be careful with allocating on stack! * }; @@ -141,7 +141,7 @@ static inline unsigned int oidhash(const struct object_id *oid) /* * struct hashmap_entry is an opaque structure representing an entry in the - * hash table, which must be used as first member of user data structures. + * hash table. * Ideally it should be followed by an int-sized member to prevent unused * memory on 64-bit systems due to alignment. */ diff --git a/merge-recursive.h b/merge-recursive.h index c2b7bb65c6..daa742568e 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -50,7 +50,7 @@ struct merge_options { * in get_directory_renames() for details */ struct dir_rename_entry { - struct hashmap_entry ent; /* must be the first member! */ + struct hashmap_entry ent; char *dir; unsigned non_unique_new_dir:1; struct strbuf new_dir; @@ -58,7 +58,7 @@ struct dir_rename_entry { }; struct collision_entry { - struct hashmap_entry ent; /* must be the first member! */ + struct hashmap_entry ent; char *target_file; struct string_list source_files; unsigned reported_already:1; diff --git a/ref-filter.c b/ref-filter.c index 0950b789e3..5c10a343c6 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -79,7 +79,7 @@ static struct expand_data { } oi, oi_deref; struct ref_to_worktree_entry { - struct hashmap_entry ent; /* must be the first member! */ + struct hashmap_entry ent; struct worktree *wt; /* key is wt->head_ref */ }; diff --git a/refs.c b/refs.c index 2d3eb40f39..1ab0bb54d3 100644 --- a/refs.c +++ b/refs.c @@ -1772,7 +1772,7 @@ int resolve_gitlink_ref(const char *submodule, const char *refname, struct ref_store_hash_entry { - struct hashmap_entry ent; /* must be the first member! */ + struct hashmap_entry ent; struct ref_store *refs; diff --git a/remote.h b/remote.h index 83e885672b..0e1d2b245b 100644 --- a/remote.h +++ b/remote.h @@ -14,7 +14,7 @@ enum { }; struct remote { - struct hashmap_entry ent; /* must be first */ + struct hashmap_entry ent; const char *name; int origin, configured_in_repo; diff --git a/sub-process.h b/sub-process.h index 0d12708b8c..e85f21fa1a 100644 --- a/sub-process.h +++ b/sub-process.h @@ -24,7 +24,7 @@ /* Members should not be accessed directly. */ struct subprocess_entry { - struct hashmap_entry ent; /* must be the first member! */ + struct hashmap_entry ent; const char *cmd; struct child_process process; }; diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index cc577c8956..f38706216f 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -5,6 +5,7 @@ struct test_entry { + int padding; /* hashmap entry no longer needs to be the first member */ struct hashmap_entry ent; /* key and value as two \0-terminated strings */ char key[FLEX_ARRAY];