# [PATCH 00/10] Friday night special: hash map cleanup 12 messages from 2017-07-01 to 2017-07-05. Participants: Stefan Beller, Junio C Hamano. Thread: https://gitlist.dev/t/46300 ## Stefan Beller, 2017-07-01 00:28 Subject: [PATCH 00/10] Friday night special: hash map cleanup Message-ID: <20170701002838.22785-1-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-1-sbeller%40google.com ``` 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, 2017-07-01 00:28 Subject: [PATCH 01/10] attr.c: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-2-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-2-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` 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 --- attr.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 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); } /* -- 2.13.0.31.g9b732c453e ``` ## Stefan Beller, 2017-07-01 00:28 Subject: [PATCH 02/10] builtin/difftool.c: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-3-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-3-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- builtin/difftool.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 03/10] builtin/describe: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-4-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-4-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- builtin/describe.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 04/10] config.c: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-5-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-5-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- config.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 09/10] name-hash.c: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-10-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-10-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- name-hash.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 10/10] t/helper/test-hashmap: use custom data instead of duplicate cmp functions Message-ID: <20170701002838.22785-11-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-11-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` 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 --- t/helper/test-hashmap.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 08/10] submodule-config.c: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-9-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-9-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- submodule-config.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 06/10] patch-ids.c: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-7-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-7-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- patch-ids.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 05/10] convert/sub-process: drop cast to hashmap_cmp_fn Message-ID: <20170701002838.22785-6-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-6-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- convert.c | 3 +-- sub-process.c | 7 +++++-- sub-process.h | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) 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, 2017-07-01 00:28 Subject: [PATCH 07/10] remote.c: drop hashmap_cmp_fn cast Message-ID: <20170701002838.22785-8-sbeller@google.com> URL: https://gitlist.dev/e/20170701002838.22785-8-sbeller%40google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Signed-off-by: Stefan Beller --- remote.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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, 2017-07-05 20:51 Subject: Re: [PATCH 00/10] Friday night special: hash map cleanup Message-ID: URL: https://gitlist.dev/e/xmqqmv8ihaj7.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20170701002838.22785-1-sbeller@google.com> ``` Stefan Beller writes: > 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. ```