test-hashmap: simplify alloc_test_entry

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 14, 2018 at 13:08 UTC 7daa825d677dcbd40724cb146f3949b7d574e8b3
1 file changed +17 -18
t/helper/test-hashmap.c
+17 -18
@@ -30,9 +30,10 @@ static int test_entry_cmp(const void *cmp_data,
30 return strcmp(e1->key, key ? key : e2->key);
31 }
32
33 -static struct test_entry *alloc_test_entry(int hash, char *key, int klen,
34 - char *value, int vlen)
33 +static struct test_entry *alloc_test_entry(int hash, char *key, char *value)
34 {
35 + size_t klen = strlen(key);
36 + size_t vlen = strlen(value);
37 struct test_entry *entry = xmalloc(st_add4(sizeof(*entry), klen, vlen, 2));
38 hashmap_entry_init(entry, hash);
39 memcpy(entry->key, key, klen + 1);
@@ -89,7 +90,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)
90 ALLOC_ARRAY(hashes, TEST_SIZE);
91 for (i = 0; i < TEST_SIZE; i++) {
92 xsnprintf(buf, sizeof(buf), "%i", i);
92 - entries[i] = alloc_test_entry(0, buf, strlen(buf), "", 0);
93 + entries[i] = alloc_test_entry(0, buf, "");
94 hashes[i] = hash(method, i, entries[i]->key);
95 }
96
@@ -155,7 +156,7 @@ int cmd_main(int argc, const char **argv)
156 /* process commands from stdin */
157 while (strbuf_getline(&line, stdin) != EOF) {
158 char *cmd, *p1 = NULL, *p2 = NULL;
158 - int l1 = 0, l2 = 0, hash = 0;
159 + int hash = 0;
160 struct test_entry *entry;
161
162 /* break line into command and up to two parameters */
@@ -166,31 +167,29 @@ int cmd_main(int argc, const char **argv)
167
168 p1 = strtok(NULL, DELIM);
169 if (p1) {
169 - l1 = strlen(p1);
170 hash = icase ? strihash(p1) : strhash(p1);
171 p2 = strtok(NULL, DELIM);
172 - if (p2)
173 - l2 = strlen(p2);
172 }
173
176 - if (!strcmp("hash", cmd) && l1) {
174 + if (!strcmp("hash", cmd) && p1) {
175
176 /* print results of different hash functions */
179 - printf("%u %u %u %u\n", strhash(p1), memhash(p1, l1),
180 - strihash(p1), memihash(p1, l1));
177 + printf("%u %u %u %u\n",
178 + strhash(p1), memhash(p1, strlen(p1)),
179 + strihash(p1), memihash(p1, strlen(p1)));
180
182 - } else if (!strcmp("add", cmd) && l1 && l2) {
181 + } else if (!strcmp("add", cmd) && p1 && p2) {
182
183 /* create entry with key = p1, value = p2 */
185 - entry = alloc_test_entry(hash, p1, l1, p2, l2);
184 + entry = alloc_test_entry(hash, p1, p2);
185
186 /* add to hashmap */
187 hashmap_add(&map, entry);
188
190 - } else if (!strcmp("put", cmd) && l1 && l2) {
189 + } else if (!strcmp("put", cmd) && p1 && p2) {
190
191 /* create entry with key = p1, value = p2 */
193 - entry = alloc_test_entry(hash, p1, l1, p2, l2);
192 + entry = alloc_test_entry(hash, p1, p2);
193
194 /* add / replace entry */
195 entry = hashmap_put(&map, entry);
@@ -199,7 +198,7 @@ int cmd_main(int argc, const char **argv)
198 puts(entry ? get_value(entry) : "NULL");
199 free(entry);
200
202 - } else if (!strcmp("get", cmd) && l1) {
201 + } else if (!strcmp("get", cmd) && p1) {
202
203 /* lookup entry in hashmap */
204 entry = hashmap_get_from_hash(&map, hash, p1);
@@ -212,7 +211,7 @@ int cmd_main(int argc, const char **argv)
211 entry = hashmap_get_next(&map, entry);
212 }
213
215 - } else if (!strcmp("remove", cmd) && l1) {
214 + } else if (!strcmp("remove", cmd) && p1) {
215
216 /* setup static key */
217 struct hashmap_entry key;
@@ -238,7 +237,7 @@ int cmd_main(int argc, const char **argv)
237 printf("%u %u\n", map.tablesize,
238 hashmap_get_size(&map));
239
241 - } else if (!strcmp("intern", cmd) && l1) {
240 + } else if (!strcmp("intern", cmd) && p1) {
241
242 /* test that strintern works */
243 const char *i1 = strintern(p1);
@@ -252,7 +251,7 @@ int cmd_main(int argc, const char **argv)
251 else
252 printf("%s\n", i1);
253
255 - } else if (!strcmp("perfhashmap", cmd) && l1 && l2) {
254 + } else if (!strcmp("perfhashmap", cmd) && p1 && p2) {
255
256 perf_hashmap(atoi(p1), atoi(p2));
257