git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Derrick Stolee <stolee@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@dunelm.org.uk>,
	git@vger.kernel.org
Subject: [PATCH v2 15/19] hashmap: use *_entry APIs for iteration
Date: Tue, 24 Sep 2019 01:03:20 +0000	[thread overview]
Message-ID: <20190924010324.22619-16-e@80x24.org> (raw)
In-Reply-To: <20190924010324.22619-1-e@80x24.org>

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 <e@80x24.org>
---
 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 47934161aa..fd81a389dd 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 a657ab0050..46837ba436 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();
 }

  parent reply	other threads:[~2019-09-24  1:04 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-26  2:43 [PATCH 00/11] hashmap: bugfixes, safety fixes, and WIP improvements Eric Wong
2019-08-26  2:43 ` [PATCH 01/11] diff: use hashmap_entry_init on moved_entry.ent Eric Wong
2019-08-27 13:31   ` Derrick Stolee
2019-08-26  2:43 ` [PATCH 02/11] packfile: use hashmap_entry in delta_base_cache_entry Eric Wong
2019-08-26  2:43 ` [PATCH 03/11] hashmap_entry_init takes "struct hashmap_entry *" Eric Wong
2019-08-27 13:35   ` Derrick Stolee
2019-08-28 15:01     ` Johannes Schindelin
2019-08-30 19:48       ` Eric Wong
2019-09-02 13:46         ` Johannes Schindelin
2019-08-26  2:43 ` [PATCH 04/11] hashmap_entry: detect improper initialization Eric Wong
2019-08-27  9:10   ` Johannes Schindelin
2019-08-27  9:49     ` Eric Wong
2019-08-27 22:16       ` Junio C Hamano
2019-08-28 15:04         ` Johannes Schindelin
2019-08-28  9:03       ` Phillip Wood
2019-08-30 19:52         ` Eric Wong
2019-09-08  7:49   ` [RFC 04/11] coccicheck: detect hashmap_entry.hash assignment Eric Wong
2019-09-09 18:15     ` Junio C Hamano
2019-08-26  2:43 ` [PATCH 05/11] hashmap_get_next takes "const struct hashmap_entry *" Eric Wong
2019-08-26  2:43 ` [PATCH 06/11] hashmap_add takes "struct " Eric Wong
2019-08-26  2:43 ` [PATCH 07/11] hashmap_get takes "const struct " Eric Wong
2019-08-26  2:43 ` [PATCH 08/11] hashmap_remove " Eric Wong
2019-08-26  2:43 ` [PATCH 09/11] hashmap_put takes "struct " Eric Wong
2019-08-26  2:43 ` [PATCH 10/11] introduce container_of macro Eric Wong
2019-08-27 14:49   ` Derrick Stolee
2019-08-28  9:11     ` Phillip Wood
2019-08-30 19:43     ` Eric Wong
2019-08-26  2:43 ` [PATCH 11/11] hashmap_get_next returns "struct hashmap_entry *" Eric Wong
2019-08-27 14:53   ` Derrick Stolee
2019-08-30 19:36     ` Eric Wong
2019-09-24  1:03 ` [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes Eric Wong
2019-09-24  1:03   ` [PATCH v2 01/19] diff: use hashmap_entry_init on moved_entry.ent Eric Wong
2019-09-24  1:03   ` [PATCH v2 02/19] coccicheck: detect hashmap_entry.hash assignment Eric Wong
2019-09-25 12:44     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 03/19] packfile: use hashmap_entry in delta_base_cache_entry Eric Wong
2019-09-24  1:03   ` [PATCH v2 04/19] hashmap_entry_init takes "struct hashmap_entry *" Eric Wong
2019-09-25 12:48     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 05/19] hashmap_get_next takes "const struct " Eric Wong
2019-09-24  1:03   ` [PATCH v2 06/19] hashmap_add takes "struct " Eric Wong
2019-09-24  1:03   ` [PATCH v2 07/19] hashmap_get takes "const struct " Eric Wong
2019-09-25 12:52     ` Derrick Stolee
2019-09-30  9:57       ` Eric Wong
2019-09-24  1:03   ` [PATCH v2 08/19] hashmap_remove " Eric Wong
2019-09-25 12:54     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 09/19] hashmap_put takes "struct " Eric Wong
2019-09-24  1:03   ` [PATCH v2 10/19] introduce container_of macro Eric Wong
2019-09-25 13:12     ` Derrick Stolee
2019-09-30 10:39       ` Eric Wong
2019-09-24  1:03   ` [PATCH v2 11/19] hashmap_get_next returns "struct hashmap_entry *" Eric Wong
2019-09-25 13:05     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 12/19] hashmap: use *_entry APIs to wrap container_of Eric Wong
2019-09-25 13:15     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 13/19] hashmap_get{,_from_hash} return "struct hashmap_entry *" Eric Wong
2019-09-24  1:03   ` [PATCH v2 14/19] hashmap_cmp_fn takes hashmap_entry params Eric Wong
2019-09-24  1:03   ` Eric Wong [this message]
2019-09-24  1:03   ` [PATCH v2 16/19] hashmap: hashmap_{put,remove} return hashmap_entry * Eric Wong
2019-09-24  1:03   ` [PATCH v2 17/19] hashmap: introduce hashmap_free_entries Eric Wong
2019-09-24  1:03   ` [PATCH v2 18/19] OFFSETOF_VAR macro to simplify hashmap iterators Eric Wong
2019-09-30 16:58     ` Junio C Hamano
2019-10-04  1:18     ` Junio C Hamano
2019-10-04  2:51       ` Eric Wong
2019-10-04  3:29         ` Junio C Hamano
2019-10-04 17:26           ` Eric Wong
2019-10-06  0:04             ` Junio C Hamano
2019-09-24  1:03   ` [PATCH v2 19/19] hashmap: remove type arg from hashmap_{get,put,remove}_entry Eric Wong
2019-09-25 12:42     ` Derrick Stolee
2019-09-25 13:30   ` [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes Derrick Stolee
2019-09-26  8:39   ` Johannes Schindelin
2019-09-30 10:01     ` Eric Wong
2019-09-26 13:48   ` Phillip Wood
2019-09-29  9:22   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190924010324.22619-16-e@80x24.org \
    --to=e@80x24.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).