git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 5/6] test-hashmap: simplify alloc_test_entry
Date: Wed, 14 Feb 2018 13:08:20 -0500	[thread overview]
Message-ID: <20180214180819.GE9919@sigill.intra.peff.net> (raw)
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


  parent reply	other threads:[~2018-02-14 18:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-14 18:03 [PATCH 0/6] minor test-hashmap fixes Jeff King
2018-02-14 18:05 ` [PATCH 1/6] test-hashmap: use ALLOC_ARRAY rather than bare malloc Jeff King
2018-02-14 18:47   ` Code AI
2018-02-14 18:06 ` [PATCH 2/6] test-hashmap: check allocation computation for overflow Jeff King
2018-02-14 18:06 ` [PATCH 3/6] test-hashmap: use xsnprintf rather than snprintf Jeff King
2018-02-14 18:07 ` [PATCH 4/6] test-hashmap: use strbuf_getline rather than fgets Jeff King
2018-02-14 18:08 ` Jeff King [this message]
2018-02-14 19:01   ` [PATCH 5/6] test-hashmap: simplify alloc_test_entry Junio C Hamano
2018-02-14 18:08 ` [PATCH 6/6] test-hashmap: use "unsigned int" for hash storage Jeff King
2018-02-14 18:41 ` [PATCH 0/6] minor test-hashmap fixes Stefan Beller
2018-02-14 18:48   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180214180819.GE9919@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).