threads / patch / 46300

patch, 10 partsFriday night special: hash map cleanup

Subject: [PATCH 00/10] Friday night special: hash map cleanup

## tl;dr

12 messages between Jul 1, 2017 and Jul 5, 2017. Diffs are folded; open one to read it.

replies: 11people: 2as markdown or json

Stefan Beller· Jul 1, 2017, 00:28 UTC · lore

This goes on top of sb/hashmap-customize-comparison. No functional impact but a pure cleanup series: No casts to hashmap_cmp_fn in the whole code base any more.

This revealed another interesting thing, which is not a bug per se: The const correctness of hashmap_cmp_fn as it requires all its void pointers to be const!

We violate this in patch-ids.c as we modify the `fndata` at some uncritical part (a part that doesn't change the equal-functionality AFAICT).

Thanks, Stefan

Stefan Beller (10):
  attr.c: drop hashmap_cmp_fn cast
  builtin/difftool.c: drop hashmap_cmp_fn cast
  builtin/describe: drop hashmap_cmp_fn cast
  config.c: drop hashmap_cmp_fn cast
  convert/sub-process: drop cast to hashmap_cmp_fn
  patch-ids.c: drop hashmap_cmp_fn cast
  remote.c: drop hashmap_cmp_fn cast
  submodule-config.c: drop hashmap_cmp_fn cast
  name-hash.c: drop hashmap_cmp_fn cast
  t/helper/test-hashmap: use custom data instead of duplicate cmp
    functions
 attr.c                  | 12 +++++++-----
 builtin/describe.c      |  9 ++++++---
 builtin/difftool.c      | 37 ++++++++++++++++++++++---------------
 config.c                | 10 ++++++----
 convert.c               |  3 +--
 name-hash.c             | 22 +++++++++++++---------
 patch-ids.c             | 14 +++++++++-----
 remote.c                | 12 ++++++++----
 sub-process.c           |  7 +++++--
 sub-process.h           |  4 ++--
 submodule-config.c      | 18 ++++++++++++------
 t/helper/test-hashmap.c | 34 ++++++++++++++++------------------
 12 files changed, 107 insertions(+), 75 deletions(-)
-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 01/10] attr.c: drop hashmap_cmp_fn cast

MAke the code more readable and less error prone by avoiding the cast of the compare function pointer in hashmap_init, but instead have the correctly named void pointers to casted to the specific data structure.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 attr.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
Show changes to attr.c +7 −5
diff --git a/attr.c b/attr.c
index 56961f0236..2f49151736 100644
--- a/attr.c
+++ b/attr.c
@@ -76,18 +76,20 @@ struct attr_hash_entry {
 };
 
 /* attr_hashmap comparison function */
-static int attr_hash_entry_cmp(void *unused_cmp_data,
-			       const struct attr_hash_entry *a,
-			       const struct attr_hash_entry *b,
-			       void *unused_keydata)
+static int attr_hash_entry_cmp(const void *unused_cmp_data,
+			       const void *entry,
+			       const void *entry_or_key,
+			       const void *unused_keydata)
 {
+	const struct attr_hash_entry *a = entry;
+	const struct attr_hash_entry *b = entry_or_key;
 	return (a->keylen != b->keylen) || strncmp(a->key, b->key, a->keylen);
 }
 
 /* Initialize an 'attr_hashmap' object */
 static void attr_hashmap_init(struct attr_hashmap *map)
 {
-	hashmap_init(&map->map, (hashmap_cmp_fn) attr_hash_entry_cmp, NULL, 0);
+	hashmap_init(&map->map, attr_hash_entry_cmp, NULL, 0);
 }
 
 /*

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 02/10] builtin/difftool.c: drop hashmap_cmp_fn cast

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/difftool.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)
Show changes to builtin/difftool.c +22 −15
diff --git a/builtin/difftool.c b/builtin/difftool.c
index a1a26ba891..8864d846f8 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -131,10 +131,12 @@ struct working_tree_entry {
 };
 
 static int working_tree_entry_cmp(const void *unused_cmp_data,
-				  struct working_tree_entry *a,
-				  struct working_tree_entry *b,
-				  void *unused_keydata)
+				  const void *entry,
+				  const void *entry_or_key,
+				  const void *unused_keydata)
 {
+	const struct working_tree_entry *a = entry;
+	const struct working_tree_entry *b = entry_or_key;
 	return strcmp(a->path, b->path);
 }
 
@@ -149,9 +151,13 @@ struct pair_entry {
 };
 
 static int pair_cmp(const void *unused_cmp_data,
-		    struct pair_entry *a, struct pair_entry *b,
-		    void *unused_keydata)
+		    const void *entry,
+		    const void *entry_or_key,
+		    const void *unused_keydata)
 {
+	const struct pair_entry *a = entry;
+	const struct pair_entry *b = entry_or_key;
+
 	return strcmp(a->path, b->path);
 }
 
@@ -179,9 +185,13 @@ struct path_entry {
 };
 
 static int path_entry_cmp(const void *unused_cmp_data,
-			  struct path_entry *a, struct path_entry *b,
-			  void *key)
+			  const void *entry,
+			  const void *entry_or_key,
+			  const void *key)
 {
+	const struct path_entry *a = entry;
+	const struct path_entry *b = entry_or_key;
+
 	return strcmp(a->path, key ? key : b->path);
 }
 
@@ -372,10 +382,9 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
 	rdir_len = rdir.len;
 	wtdir_len = wtdir.len;
 
-	hashmap_init(&working_tree_dups,
-		     (hashmap_cmp_fn)working_tree_entry_cmp, NULL, 0);
-	hashmap_init(&submodules, (hashmap_cmp_fn)pair_cmp, NULL, 0);
-	hashmap_init(&symlinks2, (hashmap_cmp_fn)pair_cmp, NULL, 0);
+	hashmap_init(&working_tree_dups, working_tree_entry_cmp, NULL, 0);
+	hashmap_init(&submodules, pair_cmp, NULL, 0);
+	hashmap_init(&symlinks2, pair_cmp, NULL, 0);
 
 	child.no_stdin = 1;
 	child.git_cmd = 1;
@@ -585,10 +594,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
 	 * in the common case of --symlinks and the difftool updating
 	 * files through the symlink.
 	 */
-	hashmap_init(&wt_modified, (hashmap_cmp_fn)path_entry_cmp,
-		     NULL, wtindex.cache_nr);
-	hashmap_init(&tmp_modified, (hashmap_cmp_fn)path_entry_cmp,
-		     NULL, wtindex.cache_nr);
+	hashmap_init(&wt_modified, path_entry_cmp, NULL, wtindex.cache_nr);
+	hashmap_init(&tmp_modified, path_entry_cmp, NULL, wtindex.cache_nr);
 
 	for (i = 0; i < wtindex.cache_nr; i++) {
 		struct hashmap_entry dummy;

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 03/10] builtin/describe: drop hashmap_cmp_fn cast

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/describe.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Show changes to builtin/describe.c +6 −3
diff --git a/builtin/describe.c b/builtin/describe.c
index 8868f00ed0..f2f2edf935 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -55,10 +55,13 @@ static const char *prio_names[] = {
 };
 
 static int commit_name_cmp(const void *unused_cmp_data,
-			   const struct commit_name *cn1,
-			   const struct commit_name *cn2,
+			   const void *entry,
+			   const void *entry_or_key,
 			   const void *peeled)
 {
+	const struct commit_name *cn1 = entry;
+	const struct commit_name *cn2 = entry_or_key;
+
 	return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);
 }
 
@@ -503,7 +506,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		return cmd_name_rev(args.argc, args.argv, prefix);
 	}
 
-	hashmap_init(&names, (hashmap_cmp_fn) commit_name_cmp, NULL, 0);
+	hashmap_init(&names, commit_name_cmp, NULL, 0);
 	for_each_rawref(get_name, NULL);
 	if (!names.size && !always)
 		die(_("No names found, cannot describe anything."));

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 04/10] config.c: drop hashmap_cmp_fn cast

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 config.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
Show changes to config.c +6 −4
diff --git a/config.c b/config.c
index 4a31e31ac3..30ff700629 100644
--- a/config.c
+++ b/config.c
@@ -1754,17 +1754,19 @@ 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 struct config_set_element *e1,
-				  const struct config_set_element *e2,
+				  const void *entry,
+				  const void *entry_or_key,
 				  const void *unused_keydata)
 {
+	const struct config_set_element *e1 = entry;
+	const struct config_set_element *e2 = entry_or_key;
+
 	return strcmp(e1->key, e2->key);
 }
 
 void git_configset_init(struct config_set *cs)
 {
-	hashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp,
-		     NULL, 0);
+	hashmap_init(&cs->config_hash, config_set_element_cmp, NULL, 0);
 	cs->hash_initialized = 1;
 	cs->list.nr = 0;
 	cs->list.alloc = 0;

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 09/10] name-hash.c: drop hashmap_cmp_fn cast

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 name-hash.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)
Show changes to name-hash.c +13 −9
diff --git a/name-hash.c b/name-hash.c
index 0e10f3eab8..bd8dc7a6a7 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -17,10 +17,14 @@ struct dir_entry {
 };
 
 static int dir_entry_cmp(const void *unused_cmp_data,
-			 const struct dir_entry *e1,
-			 const struct dir_entry *e2,
-			 const char *name)
+			 const void *entry,
+			 const void *entry_or_key,
+			 const void *keydata)
 {
+	const struct dir_entry *e1 = entry;
+	const struct dir_entry *e2 = entry_or_key;
+	const char *name = keydata;
+
 	return e1->namelen != e2->namelen || strncasecmp(e1->name,
 			name ? name : e2->name, e1->namelen);
 }
@@ -110,10 +114,12 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
 }
 
 static int cache_entry_cmp(const void *unused_cmp_data,
-			   const struct cache_entry *ce1,
-			   const struct cache_entry *ce2,
+			   const void *entry,
+			   const void *entry_or_key,
 			   const void *remove)
 {
+	const struct cache_entry *ce1 = entry;
+	const struct cache_entry *ce2 = entry_or_key;
 	/*
 	 * For remove_name_hash, find the exact entry (pointer equality); for
 	 * index_file_exists, find all entries with matching hash code and
@@ -574,10 +580,8 @@ static void lazy_init_name_hash(struct index_state *istate)
 {
 	if (istate->name_hash_initialized)
 		return;
-	hashmap_init(&istate->name_hash, (hashmap_cmp_fn) cache_entry_cmp,
-			NULL, istate->cache_nr);
-	hashmap_init(&istate->dir_hash, (hashmap_cmp_fn) dir_entry_cmp,
-			NULL, istate->cache_nr);
+	hashmap_init(&istate->name_hash, cache_entry_cmp, NULL, istate->cache_nr);
+	hashmap_init(&istate->dir_hash, dir_entry_cmp, NULL, istate->cache_nr);
 
 	if (lookup_lazy_params(istate)) {
 		hashmap_disallow_rehash(&istate->dir_hash, 1);

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 10/10] t/helper/test-hashmap: use custom data instead of duplicate cmp functions

With the new field that is passed to the compare function, we can pass through flags there instead of having multiple compare functions. Also drop the cast to hashmap_cmp_fn.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/helper/test-hashmap.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)
Show changes to t/helper/test-hashmap.c +16 −18
diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c
index 095d7395f3..93ccfbb75f 100644
--- a/t/helper/test-hashmap.c
+++ b/t/helper/test-hashmap.c
@@ -13,20 +13,20 @@ static const char *get_value(const struct test_entry *e)
 	return e->key + strlen(e->key) + 1;
 }
 
-static int test_entry_cmp(const void *unused_cmp_data,
-			  const struct test_entry *e1,
-			  const struct test_entry *e2,
-			  const char* key)
+static int test_entry_cmp(const void *cmp_data,
+			  const void *entry,
+			  const void *entry_or_key,
+			  const void* keydata)
 {
-	return strcmp(e1->key, key ? key : e2->key);
-}
-
-static int test_entry_cmp_icase(const void *unused_cmp_data,
-				const struct test_entry *e1,
-				const struct test_entry *e2,
-				const char* key)
-{
-	return strcasecmp(e1->key, key ? key : e2->key);
+	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 char* key = keydata;
+
+	if (ignore_case)
+		return strcasecmp(e1->key, key ? key : e2->key);
+	else
+		return strcmp(e1->key, key ? key : e2->key);
 }
 
 static struct test_entry *alloc_test_entry(int hash, char *key, int klen,
@@ -96,8 +96,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)
 	if (method & TEST_ADD) {
 		/* test adding to the map */
 		for (j = 0; j < rounds; j++) {
-			hashmap_init(&map, (hashmap_cmp_fn) test_entry_cmp,
-				     NULL, 0);
+			hashmap_init(&map, test_entry_cmp, NULL, 0);
 
 			/* add entries */
 			for (i = 0; i < TEST_SIZE; i++) {
@@ -109,7 +108,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)
 		}
 	} else {
 		/* test map lookups */
-		hashmap_init(&map, (hashmap_cmp_fn) test_entry_cmp, NULL, 0);
+		hashmap_init(&map, test_entry_cmp, NULL, 0);
 
 		/* fill the map (sparsely if specified) */
 		j = (method & TEST_SPARSE) ? TEST_SIZE / 10 : TEST_SIZE;
@@ -151,8 +150,7 @@ int cmd_main(int argc, const char **argv)
 
 	/* init hash map */
 	icase = argc > 1 && !strcmp("ignorecase", argv[1]);
-	hashmap_init(&map, (hashmap_cmp_fn) (icase ? test_entry_cmp_icase
-			: test_entry_cmp), NULL, 0);
+	hashmap_init(&map, test_entry_cmp, &icase, 0);
 
 	/* process commands from stdin */
 	while (fgets(line, sizeof(line), stdin)) {

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 08/10] submodule-config.c: drop hashmap_cmp_fn cast

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule-config.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
Show changes to submodule-config.c +12 −6
diff --git a/submodule-config.c b/submodule-config.c
index 0e1126183d..edc8dd04b6 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -35,27 +35,33 @@ static struct submodule_cache the_submodule_cache;
 static int is_cache_init;
 
 static int config_path_cmp(const void *unused_cmp_data,
-			   const struct submodule_entry *a,
-			   const struct submodule_entry *b,
+			   const void *entry,
+			   const void *entry_or_key,
 			   const void *unused_keydata)
 {
+	const struct submodule_entry *a = entry;
+	const struct submodule_entry *b = entry_or_key;
+
 	return strcmp(a->config->path, b->config->path) ||
 	       hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);
 }
 
 static int config_name_cmp(const void *unused_cmp_data,
-			   const struct submodule_entry *a,
-			   const struct submodule_entry *b,
+			   const void *entry,
+			   const void *entry_or_key,
 			   const void *unused_keydata)
 {
+	const struct submodule_entry *a = entry;
+	const struct submodule_entry *b = entry_or_key;
+
 	return strcmp(a->config->name, b->config->name) ||
 	       hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);
 }
 
 static void cache_init(struct submodule_cache *cache)
 {
-	hashmap_init(&cache->for_path, (hashmap_cmp_fn) config_path_cmp, NULL, 0);
-	hashmap_init(&cache->for_name, (hashmap_cmp_fn) config_name_cmp, NULL, 0);
+	hashmap_init(&cache->for_path, config_path_cmp, NULL, 0);
+	hashmap_init(&cache->for_name, config_name_cmp, NULL, 0);
 }
 
 static void free_one_config(struct submodule_entry *entry)

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 06/10] patch-ids.c: drop hashmap_cmp_fn cast

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 patch-ids.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
Show changes to patch-ids.c +9 −5
diff --git a/patch-ids.c b/patch-ids.c
index b4166b0f38..9ea523984b 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -35,11 +35,16 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
  * the side of safety.  The actual value being negative does not have
  * any significance; only that it is non-zero matters.
  */
-static int patch_id_cmp(struct diff_options *opt,
-			struct patch_id *a,
-			struct patch_id *b,
+static int patch_id_cmp(const void *cmpfn_data,
+			const void *entry,
+			const void *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;
+
 	if (is_null_oid(&a->patch_id) &&
 	    commit_patch_id(a->commit, opt, &a->patch_id, 0))
 		return error("Could not get patch ID for %s",
@@ -58,8 +63,7 @@ int init_patch_ids(struct patch_ids *ids)
 	ids->diffopts.detect_rename = 0;
 	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
 	diff_setup_done(&ids->diffopts);
-	hashmap_init(&ids->patches, (hashmap_cmp_fn)patch_id_cmp,
-		     &ids->diffopts, 256);
+	hashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256);
 	return 0;
 }
 

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 05/10] convert/sub-process: drop cast to hashmap_cmp_fn

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 convert.c     | 3 +--
 sub-process.c | 7 +++++--
 sub-process.h | 4 ++--
 3 files changed, 8 insertions(+), 6 deletions(-)
Show changes to 3 files +8 −6

convert.c, sub-process.c, sub-process.h

diff --git a/convert.c b/convert.c
index deaf0ba7b3..04966c723c 100644
--- a/convert.c
+++ b/convert.c
@@ -583,8 +583,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
 
 	if (!subprocess_map_initialized) {
 		subprocess_map_initialized = 1;
-		hashmap_init(&subprocess_map, (hashmap_cmp_fn) cmd2process_cmp,
-			     NULL, 0);
+		hashmap_init(&subprocess_map, cmd2process_cmp, NULL, 0);
 		entry = NULL;
 	} else {
 		entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd);
diff --git a/sub-process.c b/sub-process.c
index a3cfab1a9d..6cbffa4406 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -6,10 +6,13 @@
 #include "pkt-line.h"
 
 int cmd2process_cmp(const void *unused_cmp_data,
-		    const struct subprocess_entry *e1,
-		    const struct subprocess_entry *e2,
+		    const void *entry,
+		    const void *entry_or_key,
 		    const void *unused_keydata)
 {
+	const struct subprocess_entry *e1 = entry;
+	const struct subprocess_entry *e2 = entry_or_key;
+
 	return strcmp(e1->cmd, e2->cmd);
 }
 
diff --git a/sub-process.h b/sub-process.h
index 96a2cca360..8cd07a59ab 100644
--- a/sub-process.h
+++ b/sub-process.h
@@ -21,8 +21,8 @@ struct subprocess_entry {
 /* subprocess functions */
 
 extern int cmd2process_cmp(const void *unused_cmp_data,
-			   const struct subprocess_entry *e1,
-			   const struct subprocess_entry *e2,
+			   const void *e1,
+			   const void *e2,
 			   const void *unused_keydata);
 
 typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);

-- 
2.13.0.31.g9b732c453e
Stefan Beller· Jul 1, 2017, 00:28 UTC · re: Stefan Beller · lore

[PATCH 07/10] remote.c: drop hashmap_cmp_fn cast

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 remote.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
Show changes to remote.c +8 −4
diff --git a/remote.c b/remote.c
index 60d0043921..3efa358558 100644
--- a/remote.c
+++ b/remote.c
@@ -134,10 +134,14 @@ struct remotes_hash_key {
 };
 
 static int remotes_hash_cmp(const void *unused_cmp_data,
-			    const struct remote *a,
-			    const struct remote *b,
-			    const struct remotes_hash_key *key)
+			    const void *entry,
+			    const void *entry_or_key,
+			    const void *keydata)
 {
+	const struct remote *a = entry;
+	const struct remote *b = entry_or_key;
+	const struct remotes_hash_key *key = keydata;
+
 	if (key)
 		return strncmp(a->name, key->str, key->len) || a->name[key->len];
 	else
@@ -147,7 +151,7 @@ static int remotes_hash_cmp(const void *unused_cmp_data,
 static inline void init_remotes_hash(void)
 {
 	if (!remotes_hash.cmpfn)
-		hashmap_init(&remotes_hash, (hashmap_cmp_fn)remotes_hash_cmp, NULL, 0);
+		hashmap_init(&remotes_hash, remotes_hash_cmp, NULL, 0);
 }
 
 static struct remote *make_remote(const char *name, int len)

-- 
2.13.0.31.g9b732c453e
Junio C Hamano· Jul 5, 2017, 20:51 UTC · re: Stefan Beller · lore

Re: [PATCH 00/10] Friday night special: hash map cleanup

Stefan Beller <sbeller@google.com> writes:
Show 11 quoted lines
> This goes on top of sb/hashmap-customize-comparison.
> No functional impact but a pure cleanup series:
> No casts to hashmap_cmp_fn in the whole code base any more.
>
> This revealed another interesting thing, which is not a bug per se:
> The const correctness of hashmap_cmp_fn as it requires all its void
> pointers to be const!
>
> We violate this in patch-ids.c as we modify the `fndata` at some
> uncritical part (a part that doesn't change the equal-functionality
> AFAICT).

I am undecided, but perhaps we should loosen that, if some real-world user has a legitimate need to take a mutable fndata?

I dunno.  The patches look good to me.
Thanks.

← back to recent threads