[PATCH 5/6] test-hashmap: simplify alloc_test_entry
- From
Jeff King <peff@peff.net>
- Date
- Feb 14, 2018, 18:08 UTC
- Message-ID
- <20180214180819.GE9919@sigill.intra.peff.net>
- In-Reply-To
- <20180214180322.GA9190@sigill.intra.peff.net>
This function takes two ptr/len pairs, which implies that they can be arbitrary buffers. But internally, it assumes that each "ptr" is NUL-terminated at "len" (because we memcpy an extra byte to pick up the NUL terminator).
In practice this works because each caller only ever passes strlen(ptr) as the length. But let's drop the "len" parameters to make our expectations clear.
Note that we can get rid of the "l1" and "l2" variables from cmd_main() as a further cleanup, since they are now mostly used to check whether the p1 and p2 arguments are present (technically the length parameters conflated NULL with the empty string, which we no longer do, but I think that is actually an improvement).
Signed-off-by: Jeff King <peff@peff.net> --- t/helper/test-hashmap.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index 15fc4e372f..56efff36e8 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -30,9 +30,10 @@ static int test_entry_cmp(const void *cmp_data, return strcmp(e1->key, key ? key : e2->key); } -static struct test_entry *alloc_test_entry(int hash, char *key, int klen, - char *value, int vlen) +static struct test_entry *alloc_test_entry(int hash, char *key, char *value) { + size_t klen = strlen(key); + size_t vlen = strlen(value); struct test_entry *entry = xmalloc(st_add4(sizeof(*entry), klen, vlen, 2)); hashmap_entry_init(entry, hash); memcpy(entry->key, key, klen + 1); @@ -89,7 +90,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds) ALLOC_ARRAY(hashes, TEST_SIZE); for (i = 0; i < TEST_SIZE; i++) { xsnprintf(buf, sizeof(buf), "%i", i); - entries[i] = alloc_test_entry(0, buf, strlen(buf), "", 0); + entries[i] = alloc_test_entry(0, buf, ""); hashes[i] = hash(method, i, entries[i]->key); } @@ -155,7 +156,7 @@ int cmd_main(int argc, const char **argv) /* process commands from stdin */ while (strbuf_getline(&line, stdin) != EOF) { char *cmd, *p1 = NULL, *p2 = NULL; - int l1 = 0, l2 = 0, hash = 0; + int hash = 0; struct test_entry *entry; /* break line into command and up to two parameters */ @@ -166,31 +167,29 @@ int cmd_main(int argc, const char **argv) p1 = strtok(NULL, DELIM); if (p1) { - l1 = strlen(p1); hash = icase ? strihash(p1) : strhash(p1); p2 = strtok(NULL, DELIM); - if (p2) - l2 = strlen(p2); } - if (!strcmp("hash", cmd) && l1) { + if (!strcmp("hash", cmd) && p1) { /* print results of different hash functions */ - printf("%u %u %u %u\n", strhash(p1), memhash(p1, l1), - strihash(p1), memihash(p1, l1)); + printf("%u %u %u %u\n", + strhash(p1), memhash(p1, strlen(p1)), + strihash(p1), memihash(p1, strlen(p1))); - } else if (!strcmp("add", cmd) && l1 && l2) { + } else if (!strcmp("add", cmd) && p1 && p2) { /* create entry with key = p1, value = p2 */ - entry = alloc_test_entry(hash, p1, l1, p2, l2); + entry = alloc_test_entry(hash, p1, p2); /* add to hashmap */ hashmap_add(&map, entry); - } else if (!strcmp("put", cmd) && l1 && l2) { + } else if (!strcmp("put", cmd) && p1 && p2) { /* create entry with key = p1, value = p2 */ - entry = alloc_test_entry(hash, p1, l1, p2, l2); + entry = alloc_test_entry(hash, p1, p2); /* add / replace entry */ entry = hashmap_put(&map, entry); @@ -199,7 +198,7 @@ int cmd_main(int argc, const char **argv) puts(entry ? get_value(entry) : "NULL"); free(entry); - } else if (!strcmp("get", cmd) && l1) { + } else if (!strcmp("get", cmd) && p1) { /* lookup entry in hashmap */ entry = hashmap_get_from_hash(&map, hash, p1); @@ -212,7 +211,7 @@ int cmd_main(int argc, const char **argv) entry = hashmap_get_next(&map, entry); } - } else if (!strcmp("remove", cmd) && l1) { + } else if (!strcmp("remove", cmd) && p1) { /* setup static key */ struct hashmap_entry key; @@ -238,7 +237,7 @@ int cmd_main(int argc, const char **argv) printf("%u %u\n", map.tablesize, hashmap_get_size(&map)); - } else if (!strcmp("intern", cmd) && l1) { + } else if (!strcmp("intern", cmd) && p1) { /* test that strintern works */ const char *i1 = strintern(p1); @@ -252,7 +251,7 @@ int cmd_main(int argc, const char **argv) else printf("%s\n", i1); - } else if (!strcmp("perfhashmap", cmd) && l1 && l2) { + } else if (!strcmp("perfhashmap", cmd) && p1 && p2) { perf_hashmap(atoi(p1), atoi(p2));
-- 2.16.1.464.gc4bae515b7