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 <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/attr.c b/attr.c
index ca8be46e8e1e2eb1de8790b1ba24445b3539e011..9849106627ea15563f6aef39c93d41aaa42f5cb7 100644 (file)
--- 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 f33af0da9ff05a6c8644d2bd81c0abd24256b4c9..90b247abf93336240a83a159ca41b1024ff80431 100644 (file)
--- 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 8cf2cd992d3b2367b73ed22eb9a6d6fd6a347dce..1caf98f71640dd235f122dee969063fcc0bc5d2e 100644 (file)
--- 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 dd94179b687cc7e36e54798b48e81161b01f874f..f2d4d1e0f806d423ac5477a2a93c65d755b8547f 100644 (file)
--- 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 4d05dbc15a7a6f1b501874774138cb634c81b273..77ed00bfbf87f8d9505a9369f078d08fe4e66e3c 100644 (file)
--- 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 f94d9f96af152e39e9550b9a24a0152fd3311786..051de9832d3396d8c566e7bd08c5a04f4581e1ff 100644 (file)
--- 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 994609ed58a512e9621e1882ef899577a0699f4c..9ad4dc395a978fe701dc34d37d4d0b970e820d3d 100644 (file)
--- 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 4a23b9090b455740b40fd937faddfdd6f007f641..8605cb42029ac8faef3760137b564005ed0ac8f3 100644 (file)
--- 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 171d6ddb7651362f75b1c3e4b9c5604ccdb22b57..96786c724a005600c56cab9e7971e5c785e88558 100644 (file)
--- 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 34b3d54154d9217c887abcaac742be4f87db1420..3abba3a6181bdc9350b7ccc00377bda4a0e1f3bb 100644 (file)
--- 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 c86fe0f1df0b95c125eecc580ee47a0788bd9632..3cda22b657619d974d2d2d70db211ee3607189fc 100644 (file)
--- 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 8a5f866ae6a7f2757a4f91ed96207c30b3a9c8b8..5abd4a1fe7b2a80aa489ed38a6a29c84f729e387 100644 (file)
--- 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 5462acc8ecbb204f110941775a5ae696f6e6c7e0..c22855cd3894bbdc03960988205f3f95ddd277ad 100644 (file)
--- 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 6f2530dcc8ce873651596cf078ca75e34af08a0a..f89d1194ef6fdf0288f0991af729130124f949fe 100644 (file)
--- 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 9d4664d6a4791cdd2e132d09442715def280bc60..cd1b4c9736ef2c658dd3467506cbe5c1612cd4d2 100644 (file)
--- 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();