{"thread":{"id":"46300","subject":"[PATCH 00/10] Friday night special: hash map cleanup","startedAt":"2017-07-01T00:28:48Z","lastAt":"2017-07-05T20:52:05Z","messageCount":12,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"323656","messageId":"20170701002838.22785-1-sbeller@google.com","threadId":"46300","inReplyTo":null,"subject":"[PATCH 00/10] Friday night special: hash map cleanup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:28Z","receivedAt":"2017-07-01T00:28:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This goes on top of sb/hashmap-customize-comparison.\nNo functional impact but a pure cleanup series:\nNo casts to hashmap_cmp_fn in the whole code base any more.\n\nThis revealed another interesting thing, which is not a bug per se:\nThe const correctness of hashmap_cmp_fn as it requires all its void\npointers to be const!\n\nWe violate this in patch-ids.c as we modify the `fndata` at some\nuncritical part (a part that doesn't change the equal-functionality\nAFAICT).\n\nThanks,\nStefan\n\nStefan Beller (10):\n  attr.c: drop hashmap_cmp_fn cast\n  builtin/difftool.c: drop hashmap_cmp_fn cast\n  builtin/describe: drop hashmap_cmp_fn cast\n  config.c: drop hashmap_cmp_fn cast\n  convert/sub-process: drop cast to hashmap_cmp_fn\n  patch-ids.c: drop hashmap_cmp_fn cast\n  remote.c: drop hashmap_cmp_fn cast\n  submodule-config.c: drop hashmap_cmp_fn cast\n  name-hash.c: drop hashmap_cmp_fn cast\n  t/helper/test-hashmap: use custom data instead of duplicate cmp\n    functions\n\n attr.c                  | 12 +++++++-----\n builtin/describe.c      |  9 ++++++---\n builtin/difftool.c      | 37 ++++++++++++++++++++++---------------\n config.c                | 10 ++++++----\n convert.c               |  3 +--\n name-hash.c             | 22 +++++++++++++---------\n patch-ids.c             | 14 +++++++++-----\n remote.c                | 12 ++++++++----\n sub-process.c           |  7 +++++--\n sub-process.h           |  4 ++--\n submodule-config.c      | 18 ++++++++++++------\n t/helper/test-hashmap.c | 34 ++++++++++++++++------------------\n 12 files changed, 107 insertions(+), 75 deletions(-)\n\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323657","messageId":"20170701002838.22785-2-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 01/10] attr.c: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:29Z","receivedAt":"2017-07-01T00:28:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"MAke the code more readable and less error prone by avoiding the cast\nof the compare function pointer in hashmap_init, but instead have the\ncorrectly named void pointers to casted to the specific data structure.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n attr.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 56961f0236..2f49151736 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -76,18 +76,20 @@ struct attr_hash_entry {\n };\n \n /* attr_hashmap comparison function */\n-static int attr_hash_entry_cmp(void *unused_cmp_data,\n-\t\t\t       const struct attr_hash_entry *a,\n-\t\t\t       const struct attr_hash_entry *b,\n-\t\t\t       void *unused_keydata)\n+static int attr_hash_entry_cmp(const void *unused_cmp_data,\n+\t\t\t       const void *entry,\n+\t\t\t       const void *entry_or_key,\n+\t\t\t       const void *unused_keydata)\n {\n+\tconst struct attr_hash_entry *a = entry;\n+\tconst struct attr_hash_entry *b = entry_or_key;\n \treturn (a->keylen != b->keylen) || strncmp(a->key, b->key, a->keylen);\n }\n \n /* Initialize an 'attr_hashmap' object */\n static void attr_hashmap_init(struct attr_hashmap *map)\n {\n-\thashmap_init(&map->map, (hashmap_cmp_fn) attr_hash_entry_cmp, NULL, 0);\n+\thashmap_init(&map->map, attr_hash_entry_cmp, NULL, 0);\n }\n \n /*\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323658","messageId":"20170701002838.22785-3-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 02/10] builtin/difftool.c: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:30Z","receivedAt":"2017-07-01T00:28:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/difftool.c | 37 ++++++++++++++++++++++---------------\n 1 file changed, 22 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex a1a26ba891..8864d846f8 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -131,10 +131,12 @@ struct working_tree_entry {\n };\n \n static int working_tree_entry_cmp(const void *unused_cmp_data,\n-\t\t\t\t  struct working_tree_entry *a,\n-\t\t\t\t  struct working_tree_entry *b,\n-\t\t\t\t  void *unused_keydata)\n+\t\t\t\t  const void *entry,\n+\t\t\t\t  const void *entry_or_key,\n+\t\t\t\t  const void *unused_keydata)\n {\n+\tconst struct working_tree_entry *a = entry;\n+\tconst struct working_tree_entry *b = entry_or_key;\n \treturn strcmp(a->path, b->path);\n }\n \n@@ -149,9 +151,13 @@ struct pair_entry {\n };\n \n static int pair_cmp(const void *unused_cmp_data,\n-\t\t    struct pair_entry *a, struct pair_entry *b,\n-\t\t    void *unused_keydata)\n+\t\t    const void *entry,\n+\t\t    const void *entry_or_key,\n+\t\t    const void *unused_keydata)\n {\n+\tconst struct pair_entry *a = entry;\n+\tconst struct pair_entry *b = entry_or_key;\n+\n \treturn strcmp(a->path, b->path);\n }\n \n@@ -179,9 +185,13 @@ struct path_entry {\n };\n \n static int path_entry_cmp(const void *unused_cmp_data,\n-\t\t\t  struct path_entry *a, struct path_entry *b,\n-\t\t\t  void *key)\n+\t\t\t  const void *entry,\n+\t\t\t  const void *entry_or_key,\n+\t\t\t  const void *key)\n {\n+\tconst struct path_entry *a = entry;\n+\tconst struct path_entry *b = entry_or_key;\n+\n \treturn strcmp(a->path, key ? key : b->path);\n }\n \n@@ -372,10 +382,9 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \trdir_len = rdir.len;\n \twtdir_len = wtdir.len;\n \n-\thashmap_init(&working_tree_dups,\n-\t\t     (hashmap_cmp_fn)working_tree_entry_cmp, NULL, 0);\n-\thashmap_init(&submodules, (hashmap_cmp_fn)pair_cmp, NULL, 0);\n-\thashmap_init(&symlinks2, (hashmap_cmp_fn)pair_cmp, NULL, 0);\n+\thashmap_init(&working_tree_dups, working_tree_entry_cmp, NULL, 0);\n+\thashmap_init(&submodules, pair_cmp, NULL, 0);\n+\thashmap_init(&symlinks2, pair_cmp, NULL, 0);\n \n \tchild.no_stdin = 1;\n \tchild.git_cmd = 1;\n@@ -585,10 +594,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,\n \t * in the common case of --symlinks and the difftool updating\n \t * files through the symlink.\n \t */\n-\thashmap_init(&wt_modified, (hashmap_cmp_fn)path_entry_cmp,\n-\t\t     NULL, wtindex.cache_nr);\n-\thashmap_init(&tmp_modified, (hashmap_cmp_fn)path_entry_cmp,\n-\t\t     NULL, wtindex.cache_nr);\n+\thashmap_init(&wt_modified, path_entry_cmp, NULL, wtindex.cache_nr);\n+\thashmap_init(&tmp_modified, path_entry_cmp, NULL, wtindex.cache_nr);\n \n \tfor (i = 0; i < wtindex.cache_nr; i++) {\n \t\tstruct hashmap_entry dummy;\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323659","messageId":"20170701002838.22785-4-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 03/10] builtin/describe: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:31Z","receivedAt":"2017-07-01T00:28:53Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/describe.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 8868f00ed0..f2f2edf935 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -55,10 +55,13 @@ static const char *prio_names[] = {\n };\n \n static int commit_name_cmp(const void *unused_cmp_data,\n-\t\t\t   const struct commit_name *cn1,\n-\t\t\t   const struct commit_name *cn2,\n+\t\t\t   const void *entry,\n+\t\t\t   const void *entry_or_key,\n \t\t\t   const void *peeled)\n {\n+\tconst struct commit_name *cn1 = entry;\n+\tconst struct commit_name *cn2 = entry_or_key;\n+\n \treturn oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);\n }\n \n@@ -503,7 +506,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\treturn cmd_name_rev(args.argc, args.argv, prefix);\n \t}\n \n-\thashmap_init(&names, (hashmap_cmp_fn) commit_name_cmp, NULL, 0);\n+\thashmap_init(&names, commit_name_cmp, NULL, 0);\n \tfor_each_rawref(get_name, NULL);\n \tif (!names.size && !always)\n \t\tdie(_(\"No names found, cannot describe anything.\"));\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323660","messageId":"20170701002838.22785-5-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 04/10] config.c: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:32Z","receivedAt":"2017-07-01T00:28:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n config.c | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 4a31e31ac3..30ff700629 100644\n--- a/config.c\n+++ b/config.c\n@@ -1754,17 +1754,19 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha\n }\n \n static int config_set_element_cmp(const void *unused_cmp_data,\n-\t\t\t\t  const struct config_set_element *e1,\n-\t\t\t\t  const struct config_set_element *e2,\n+\t\t\t\t  const void *entry,\n+\t\t\t\t  const void *entry_or_key,\n \t\t\t\t  const void *unused_keydata)\n {\n+\tconst struct config_set_element *e1 = entry;\n+\tconst struct config_set_element *e2 = entry_or_key;\n+\n \treturn strcmp(e1->key, e2->key);\n }\n \n void git_configset_init(struct config_set *cs)\n {\n-\thashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp,\n-\t\t     NULL, 0);\n+\thashmap_init(&cs->config_hash, config_set_element_cmp, NULL, 0);\n \tcs->hash_initialized = 1;\n \tcs->list.nr = 0;\n \tcs->list.alloc = 0;\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323661","messageId":"20170701002838.22785-10-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 09/10] name-hash.c: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:37Z","receivedAt":"2017-07-01T00:28:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n name-hash.c | 22 +++++++++++++---------\n 1 file changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex 0e10f3eab8..bd8dc7a6a7 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -17,10 +17,14 @@ struct dir_entry {\n };\n \n static int dir_entry_cmp(const void *unused_cmp_data,\n-\t\t\t const struct dir_entry *e1,\n-\t\t\t const struct dir_entry *e2,\n-\t\t\t const char *name)\n+\t\t\t const void *entry,\n+\t\t\t const void *entry_or_key,\n+\t\t\t const void *keydata)\n {\n+\tconst struct dir_entry *e1 = entry;\n+\tconst struct dir_entry *e2 = entry_or_key;\n+\tconst char *name = keydata;\n+\n \treturn e1->namelen != e2->namelen || strncasecmp(e1->name,\n \t\t\tname ? name : e2->name, e1->namelen);\n }\n@@ -110,10 +114,12 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)\n }\n \n static int cache_entry_cmp(const void *unused_cmp_data,\n-\t\t\t   const struct cache_entry *ce1,\n-\t\t\t   const struct cache_entry *ce2,\n+\t\t\t   const void *entry,\n+\t\t\t   const void *entry_or_key,\n \t\t\t   const void *remove)\n {\n+\tconst struct cache_entry *ce1 = entry;\n+\tconst struct cache_entry *ce2 = entry_or_key;\n \t/*\n \t * For remove_name_hash, find the exact entry (pointer equality); for\n \t * index_file_exists, find all entries with matching hash code and\n@@ -574,10 +580,8 @@ static void lazy_init_name_hash(struct index_state *istate)\n {\n \tif (istate->name_hash_initialized)\n \t\treturn;\n-\thashmap_init(&istate->name_hash, (hashmap_cmp_fn) cache_entry_cmp,\n-\t\t\tNULL, istate->cache_nr);\n-\thashmap_init(&istate->dir_hash, (hashmap_cmp_fn) dir_entry_cmp,\n-\t\t\tNULL, istate->cache_nr);\n+\thashmap_init(&istate->name_hash, cache_entry_cmp, NULL, istate->cache_nr);\n+\thashmap_init(&istate->dir_hash, dir_entry_cmp, NULL, istate->cache_nr);\n \n \tif (lookup_lazy_params(istate)) {\n \t\thashmap_disallow_rehash(&istate->dir_hash, 1);\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323662","messageId":"20170701002838.22785-11-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 10/10] t/helper/test-hashmap: use custom data instead of duplicate cmp functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:38Z","receivedAt":"2017-07-01T00:28:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"With the new field that is passed to the compare function, we can pass\nthrough flags there instead of having multiple compare functions.\nAlso drop the cast to hashmap_cmp_fn.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/helper/test-hashmap.c | 34 ++++++++++++++++------------------\n 1 file changed, 16 insertions(+), 18 deletions(-)\n\ndiff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c\nindex 095d7395f3..93ccfbb75f 100644\n--- a/t/helper/test-hashmap.c\n+++ b/t/helper/test-hashmap.c\n@@ -13,20 +13,20 @@ static const char *get_value(const struct test_entry *e)\n \treturn e->key + strlen(e->key) + 1;\n }\n \n-static int test_entry_cmp(const void *unused_cmp_data,\n-\t\t\t  const struct test_entry *e1,\n-\t\t\t  const struct test_entry *e2,\n-\t\t\t  const char* key)\n+static int test_entry_cmp(const void *cmp_data,\n+\t\t\t  const void *entry,\n+\t\t\t  const void *entry_or_key,\n+\t\t\t  const void* keydata)\n {\n-\treturn strcmp(e1->key, key ? key : e2->key);\n-}\n-\n-static int test_entry_cmp_icase(const void *unused_cmp_data,\n-\t\t\t\tconst struct test_entry *e1,\n-\t\t\t\tconst struct test_entry *e2,\n-\t\t\t\tconst char* key)\n-{\n-\treturn strcasecmp(e1->key, key ? key : e2->key);\n+\tconst int ignore_case = cmp_data ? *((int*)cmp_data) : 0;\n+\tconst struct test_entry *e1 = entry;\n+\tconst struct test_entry *e2 = entry_or_key;\n+\tconst char* key = keydata;\n+\n+\tif (ignore_case)\n+\t\treturn strcasecmp(e1->key, key ? key : e2->key);\n+\telse\n+\t\treturn strcmp(e1->key, key ? key : e2->key);\n }\n \n static struct test_entry *alloc_test_entry(int hash, char *key, int klen,\n@@ -96,8 +96,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)\n \tif (method & TEST_ADD) {\n \t\t/* test adding to the map */\n \t\tfor (j = 0; j < rounds; j++) {\n-\t\t\thashmap_init(&map, (hashmap_cmp_fn) test_entry_cmp,\n-\t\t\t\t     NULL, 0);\n+\t\t\thashmap_init(&map, test_entry_cmp, NULL, 0);\n \n \t\t\t/* add entries */\n \t\t\tfor (i = 0; i < TEST_SIZE; i++) {\n@@ -109,7 +108,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)\n \t\t}\n \t} else {\n \t\t/* test map lookups */\n-\t\thashmap_init(&map, (hashmap_cmp_fn) test_entry_cmp, NULL, 0);\n+\t\thashmap_init(&map, test_entry_cmp, NULL, 0);\n \n \t\t/* fill the map (sparsely if specified) */\n \t\tj = (method & TEST_SPARSE) ? TEST_SIZE / 10 : TEST_SIZE;\n@@ -151,8 +150,7 @@ int cmd_main(int argc, const char **argv)\n \n \t/* init hash map */\n \ticase = argc > 1 && !strcmp(\"ignorecase\", argv[1]);\n-\thashmap_init(&map, (hashmap_cmp_fn) (icase ? test_entry_cmp_icase\n-\t\t\t: test_entry_cmp), NULL, 0);\n+\thashmap_init(&map, test_entry_cmp, &icase, 0);\n \n \t/* process commands from stdin */\n \twhile (fgets(line, sizeof(line), stdin)) {\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323663","messageId":"20170701002838.22785-9-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 08/10] submodule-config.c: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:36Z","receivedAt":"2017-07-01T00:29:03Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 18 ++++++++++++------\n 1 file changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 0e1126183d..edc8dd04b6 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -35,27 +35,33 @@ static struct submodule_cache the_submodule_cache;\n static int is_cache_init;\n \n static int config_path_cmp(const void *unused_cmp_data,\n-\t\t\t   const struct submodule_entry *a,\n-\t\t\t   const struct submodule_entry *b,\n+\t\t\t   const void *entry,\n+\t\t\t   const void *entry_or_key,\n \t\t\t   const void *unused_keydata)\n {\n+\tconst struct submodule_entry *a = entry;\n+\tconst struct submodule_entry *b = entry_or_key;\n+\n \treturn strcmp(a->config->path, b->config->path) ||\n \t       hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);\n }\n \n static int config_name_cmp(const void *unused_cmp_data,\n-\t\t\t   const struct submodule_entry *a,\n-\t\t\t   const struct submodule_entry *b,\n+\t\t\t   const void *entry,\n+\t\t\t   const void *entry_or_key,\n \t\t\t   const void *unused_keydata)\n {\n+\tconst struct submodule_entry *a = entry;\n+\tconst struct submodule_entry *b = entry_or_key;\n+\n \treturn strcmp(a->config->name, b->config->name) ||\n \t       hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);\n }\n \n static void cache_init(struct submodule_cache *cache)\n {\n-\thashmap_init(&cache->for_path, (hashmap_cmp_fn) config_path_cmp, NULL, 0);\n-\thashmap_init(&cache->for_name, (hashmap_cmp_fn) config_name_cmp, NULL, 0);\n+\thashmap_init(&cache->for_path, config_path_cmp, NULL, 0);\n+\thashmap_init(&cache->for_name, config_name_cmp, NULL, 0);\n }\n \n static void free_one_config(struct submodule_entry *entry)\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323664","messageId":"20170701002838.22785-7-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 06/10] patch-ids.c: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:34Z","receivedAt":"2017-07-01T00:29:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n patch-ids.c | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/patch-ids.c b/patch-ids.c\nindex b4166b0f38..9ea523984b 100644\n--- a/patch-ids.c\n+++ b/patch-ids.c\n@@ -35,11 +35,16 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,\n  * the side of safety.  The actual value being negative does not have\n  * any significance; only that it is non-zero matters.\n  */\n-static int patch_id_cmp(struct diff_options *opt,\n-\t\t\tstruct patch_id *a,\n-\t\t\tstruct patch_id *b,\n+static int patch_id_cmp(const void *cmpfn_data,\n+\t\t\tconst void *entry,\n+\t\t\tconst void *entry_or_key,\n \t\t\tconst void *unused_keydata)\n {\n+\t/* NEEDSWORK: const correctness? */\n+\tstruct diff_options *opt = (void*)cmpfn_data;\n+\tstruct patch_id *a = (void*)entry;\n+\tstruct patch_id *b = (void*)entry_or_key;\n+\n \tif (is_null_oid(&a->patch_id) &&\n \t    commit_patch_id(a->commit, opt, &a->patch_id, 0))\n \t\treturn error(\"Could not get patch ID for %s\",\n@@ -58,8 +63,7 @@ int init_patch_ids(struct patch_ids *ids)\n \tids->diffopts.detect_rename = 0;\n \tDIFF_OPT_SET(&ids->diffopts, RECURSIVE);\n \tdiff_setup_done(&ids->diffopts);\n-\thashmap_init(&ids->patches, (hashmap_cmp_fn)patch_id_cmp,\n-\t\t     &ids->diffopts, 256);\n+\thashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256);\n \treturn 0;\n }\n \n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323665","messageId":"20170701002838.22785-6-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 05/10] convert/sub-process: drop cast to hashmap_cmp_fn","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:33Z","receivedAt":"2017-07-01T00:29:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n convert.c     | 3 +--\n sub-process.c | 7 +++++--\n sub-process.h | 4 ++--\n 3 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex deaf0ba7b3..04966c723c 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -583,8 +583,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \n \tif (!subprocess_map_initialized) {\n \t\tsubprocess_map_initialized = 1;\n-\t\thashmap_init(&subprocess_map, (hashmap_cmp_fn) cmd2process_cmp,\n-\t\t\t     NULL, 0);\n+\t\thashmap_init(&subprocess_map, cmd2process_cmp, NULL, 0);\n \t\tentry = NULL;\n \t} else {\n \t\tentry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd);\ndiff --git a/sub-process.c b/sub-process.c\nindex a3cfab1a9d..6cbffa4406 100644\n--- a/sub-process.c\n+++ b/sub-process.c\n@@ -6,10 +6,13 @@\n #include \"pkt-line.h\"\n \n int cmd2process_cmp(const void *unused_cmp_data,\n-\t\t    const struct subprocess_entry *e1,\n-\t\t    const struct subprocess_entry *e2,\n+\t\t    const void *entry,\n+\t\t    const void *entry_or_key,\n \t\t    const void *unused_keydata)\n {\n+\tconst struct subprocess_entry *e1 = entry;\n+\tconst struct subprocess_entry *e2 = entry_or_key;\n+\n \treturn strcmp(e1->cmd, e2->cmd);\n }\n \ndiff --git a/sub-process.h b/sub-process.h\nindex 96a2cca360..8cd07a59ab 100644\n--- a/sub-process.h\n+++ b/sub-process.h\n@@ -21,8 +21,8 @@ struct subprocess_entry {\n /* subprocess functions */\n \n extern int cmd2process_cmp(const void *unused_cmp_data,\n-\t\t\t   const struct subprocess_entry *e1,\n-\t\t\t   const struct subprocess_entry *e2,\n+\t\t\t   const void *e1,\n+\t\t\t   const void *e2,\n \t\t\t   const void *unused_keydata);\n \n typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323666","messageId":"20170701002838.22785-8-sbeller@google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"[PATCH 07/10] remote.c: drop hashmap_cmp_fn cast","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-01T00:28:35Z","receivedAt":"2017-07-01T00:29:12Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n remote.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 60d0043921..3efa358558 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -134,10 +134,14 @@ struct remotes_hash_key {\n };\n \n static int remotes_hash_cmp(const void *unused_cmp_data,\n-\t\t\t    const struct remote *a,\n-\t\t\t    const struct remote *b,\n-\t\t\t    const struct remotes_hash_key *key)\n+\t\t\t    const void *entry,\n+\t\t\t    const void *entry_or_key,\n+\t\t\t    const void *keydata)\n {\n+\tconst struct remote *a = entry;\n+\tconst struct remote *b = entry_or_key;\n+\tconst struct remotes_hash_key *key = keydata;\n+\n \tif (key)\n \t\treturn strncmp(a->name, key->str, key->len) || a->name[key->len];\n \telse\n@@ -147,7 +151,7 @@ static int remotes_hash_cmp(const void *unused_cmp_data,\n static inline void init_remotes_hash(void)\n {\n \tif (!remotes_hash.cmpfn)\n-\t\thashmap_init(&remotes_hash, (hashmap_cmp_fn)remotes_hash_cmp, NULL, 0);\n+\t\thashmap_init(&remotes_hash, remotes_hash_cmp, NULL, 0);\n }\n \n static struct remote *make_remote(const char *name, int len)\n-- \n2.13.0.31.g9b732c453e\n\n"},{"id":"323867","messageId":"xmqqmv8ihaj7.fsf@gitster.mtv.corp.google.com","threadId":"46300","inReplyTo":"20170701002838.22785-1-sbeller@google.com","subject":"Re: [PATCH 00/10] Friday night special: hash map cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-05T20:51:56Z","receivedAt":"2017-07-05T20:52:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> This goes on top of sb/hashmap-customize-comparison.\n> No functional impact but a pure cleanup series:\n> No casts to hashmap_cmp_fn in the whole code base any more.\n>\n> This revealed another interesting thing, which is not a bug per se:\n> The const correctness of hashmap_cmp_fn as it requires all its void\n> pointers to be const!\n>\n> We violate this in patch-ids.c as we modify the `fndata` at some\n> uncritical part (a part that doesn't change the equal-functionality\n> AFAICT).\n\nI am undecided, but perhaps we should loosen that, if some real-world\nuser has a legitimate need to take a mutable fndata?\n\nI dunno.  The patches look good to me.\n\nThanks.\n"}]}