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 <[email protected]>
Reviewed-by: Derrick Stolee <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: Eric Wong

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;

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);

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);

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);

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.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) {

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;
 

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

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.

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);