Merge branch 'sb/hashmap-cleanup'

Many uses of comparision callback function the hashmap API uses
cast the callback function type when registering it to
hashmap_init(), which defeats the compile time type checking when
the callback interface changes (e.g. gaining more parameters).
The callback implementations have been updated to take "void *"
pointers and cast them to the type they expect instead.

* sb/hashmap-cleanup:
  t/helper/test-hashmap: use custom data instead of duplicate cmp functions
  name-hash.c: drop hashmap_cmp_fn cast
  submodule-config.c: drop hashmap_cmp_fn cast
  remote.c: drop hashmap_cmp_fn cast
  patch-ids.c: drop hashmap_cmp_fn cast
  convert/sub-process: drop cast to hashmap_cmp_fn
  config.c: drop hashmap_cmp_fn cast
  builtin/describe: drop hashmap_cmp_fn cast
  builtin/difftool.c: drop hashmap_cmp_fn cast
  attr.c: drop hashmap_cmp_fn cast

12 files changed, 107 insertions, 75 deletions

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);
 }
 
 /*
diff --git a/builtin/describe.c b/builtin/describe.c
index 89ea1cdd60..9c13c6817b 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."));
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;
diff --git a/config.c b/config.c
index e04500d13b..9ec8c44882 100644
--- a/config.c
+++ b/config.c
@@ -1719,17 +1719,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;
diff --git a/convert.c b/convert.c
index 972bf8aec2..dbdbb24e4d 100644
--- a/convert.c
+++ b/convert.c
@@ -625,8 +625,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/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);
diff --git a/patch-ids.c b/patch-ids.c
index b4166b0f38..7a583b3011 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;
 }
 
diff --git a/remote.c b/remote.c
index 6a1be31bb1..43c317e4e9 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)
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);
diff --git a/submodule-config.c b/submodule-config.c
index d48403f25e..bede338c85 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -35,19 +35,25 @@ enum lookup_type {
 };
 
 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);
 }
@@ -59,8 +65,8 @@ static struct submodule_cache *submodule_cache_alloc(void)
 
 static void submodule_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);
 	cache->initialized = 1;
 }
 
diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c
index 095d7395f3..6004c81f0b 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)) {