git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH 04/11] hashmap_entry: detect improper initialization
Date: Mon, 26 Aug 2019 02:43:25 +0000	[thread overview]
Message-ID: <20190826024332.3403-5-e@80x24.org> (raw)
In-Reply-To: <20190826024332.3403-1-e@80x24.org>

By renaming the "hash" field to "_hash", it's easy to spot
improper initialization of hashmap_entry structs which
can leave "hashmap_entry.next" uninitialized.

Signed-off-by: Eric Wong <e@80x24.org>
---
 builtin/fast-export.c               | 5 +++--
 hashmap.c                           | 9 +++++----
 hashmap.h                           | 4 ++--
 name-hash.c                         | 9 +++++----
 remote.c                            | 6 ++++--
 t/helper/test-lazy-init-name-hash.c | 4 ++--
 6 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 287dbd24a3..f30a92a4d3 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -144,18 +144,19 @@ static const void *anonymize_mem(struct hashmap *map,
 				 const void *orig, size_t *len)
 {
 	struct anonymized_entry key, *ret;
+	unsigned int hash = memhash(orig, *len);
 
 	if (!map->cmpfn)
 		hashmap_init(map, anonymized_entry_cmp, NULL, 0);
 
-	hashmap_entry_init(&key.hash, memhash(orig, *len));
+	hashmap_entry_init(&key.hash, hash);
 	key.orig = orig;
 	key.orig_len = *len;
 	ret = hashmap_get(map, &key, NULL);
 
 	if (!ret) {
 		ret = xmalloc(sizeof(*ret));
-		hashmap_entry_init(&ret->hash, key.hash.hash);
+		hashmap_entry_init(&ret->hash, hash);
 		ret->orig = xstrdup(orig);
 		ret->orig_len = *len;
 		ret->anon = generate(orig, len);
diff --git a/hashmap.c b/hashmap.c
index 6818c65174..777beda347 100644
--- a/hashmap.c
+++ b/hashmap.c
@@ -96,14 +96,14 @@ static inline int entry_equals(const struct hashmap *map,
 		const void *keydata)
 {
 	return (e1 == e2) ||
-	       (e1->hash == e2->hash &&
+	       (e1->_hash == e2->_hash &&
 		!map->cmpfn(map->cmpfn_data, e1, e2, keydata));
 }
 
 static inline unsigned int bucket(const struct hashmap *map,
 		const struct hashmap_entry *key)
 {
-	return key->hash & (map->tablesize - 1);
+	return key->_hash & (map->tablesize - 1);
 }
 
 int hashmap_bucket(const struct hashmap *map, unsigned int hash)
@@ -287,19 +287,20 @@ const void *memintern(const void *data, size_t len)
 {
 	static struct hashmap map;
 	struct pool_entry key, *e;
+	unsigned int hash = memhash(data, len);
 
 	/* initialize string pool hashmap */
 	if (!map.tablesize)
 		hashmap_init(&map, (hashmap_cmp_fn) pool_entry_cmp, NULL, 0);
 
 	/* lookup interned string in pool */
-	hashmap_entry_init(&key.ent, memhash(data, len));
+	hashmap_entry_init(&key.ent, hash);
 	key.len = len;
 	e = hashmap_get(&map, &key, data);
 	if (!e) {
 		/* not found: create it */
 		FLEX_ALLOC_MEM(e, data, data, len);
-		hashmap_entry_init(&e->ent, key.ent.hash);
+		hashmap_entry_init(&e->ent, hash);
 		e->len = len;
 		hashmap_add(&map, e);
 	}
diff --git a/hashmap.h b/hashmap.h
index 3d7939c291..d635e0815a 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -145,7 +145,7 @@ struct hashmap_entry {
 	struct hashmap_entry *next;
 
 	/* entry's hash code */
-	unsigned int hash;
+	unsigned int _hash;
 };
 
 /*
@@ -247,7 +247,7 @@ void hashmap_free(struct hashmap *map, int free_entries);
 static inline void
 hashmap_entry_init(struct hashmap_entry *e, unsigned int hash)
 {
-	e->hash = hash;
+	e->_hash = hash;
 	e->next = NULL;
 }
 
diff --git a/name-hash.c b/name-hash.c
index 1ce1417f7e..8b33c5cb59 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -268,7 +268,7 @@ static struct dir_entry *hash_dir_entry_with_parent_and_prefix(
 	assert((parent != NULL) ^ (strchr(prefix->buf, '/') == NULL));
 
 	if (parent)
-		hash = memihash_cont(parent->ent.hash,
+		hash = memihash_cont(parent->ent._hash,
 			prefix->buf + parent->namelen,
 			prefix->len - parent->namelen);
 	else
@@ -289,7 +289,8 @@ static struct dir_entry *hash_dir_entry_with_parent_and_prefix(
 			unlock_dir_mutex(lock_nr);
 
 			/* All I really need here is an InterlockedIncrement(&(parent->nr)) */
-			lock_nr = compute_dir_lock_nr(&istate->dir_hash, parent->ent.hash);
+			lock_nr = compute_dir_lock_nr(&istate->dir_hash,
+							parent->ent._hash);
 			lock_dir_mutex(lock_nr);
 			parent->nr++;
 		}
@@ -427,10 +428,10 @@ static int handle_range_1(
 		lazy_entries[k].dir = parent;
 		if (parent) {
 			lazy_entries[k].hash_name = memihash_cont(
-				parent->ent.hash,
+				parent->ent._hash,
 				ce_k->name + parent->namelen,
 				ce_namelen(ce_k) - parent->namelen);
-			lazy_entries[k].hash_dir = parent->ent.hash;
+			lazy_entries[k].hash_dir = parent->ent._hash;
 		} else {
 			lazy_entries[k].hash_name = memihash(ce_k->name, ce_namelen(ce_k));
 		}
diff --git a/remote.c b/remote.c
index bd81cb71bc..dc09172e9d 100644
--- a/remote.c
+++ b/remote.c
@@ -136,6 +136,7 @@ static struct remote *make_remote(const char *name, int len)
 	struct remote *ret, *replaced;
 	struct remotes_hash_key lookup;
 	struct hashmap_entry lookup_entry;
+	unsigned int hash;
 
 	if (!len)
 		len = strlen(name);
@@ -143,7 +144,8 @@ static struct remote *make_remote(const char *name, int len)
 	init_remotes_hash();
 	lookup.str = name;
 	lookup.len = len;
-	hashmap_entry_init(&lookup_entry, memhash(name, len));
+	hash = memhash(name, len);
+	hashmap_entry_init(&lookup_entry, hash);
 
 	if ((ret = hashmap_get(&remotes_hash, &lookup_entry, &lookup)) != NULL)
 		return ret;
@@ -158,7 +160,7 @@ static struct remote *make_remote(const char *name, int len)
 	ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
 	remotes[remotes_nr++] = ret;
 
-	hashmap_entry_init(&ret->ent, lookup_entry.hash);
+	hashmap_entry_init(&ret->ent, hash);
 	replaced = hashmap_put(&remotes_hash, ret);
 	assert(replaced == NULL);  /* no previous entry overwritten */
 	return ret;
diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c
index b99a37080d..d01ea0e526 100644
--- a/t/helper/test-lazy-init-name-hash.c
+++ b/t/helper/test-lazy-init-name-hash.c
@@ -43,13 +43,13 @@ static void dump_run(void)
 
 	dir = hashmap_iter_first(&the_index.dir_hash, &iter_dir);
 	while (dir) {
-		printf("dir %08x %7d %s\n", dir->ent.hash, dir->nr, dir->name);
+		printf("dir %08x %7d %s\n", dir->ent._hash, dir->nr, dir->name);
 		dir = hashmap_iter_next(&iter_dir);
 	}
 
 	ce = hashmap_iter_first(&the_index.name_hash, &iter_cache);
 	while (ce) {
-		printf("name %08x %s\n", ce->ent.hash, ce->name);
+		printf("name %08x %s\n", ce->ent._hash, ce->name);
 		ce = hashmap_iter_next(&iter_cache);
 	}
 
-- 
EW


  parent reply	other threads:[~2019-08-26  2:43 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-26  2:43 [PATCH 00/11] hashmap: bugfixes, safety fixes, and WIP improvements Eric Wong
2019-08-26  2:43 ` [PATCH 01/11] diff: use hashmap_entry_init on moved_entry.ent Eric Wong
2019-08-27 13:31   ` Derrick Stolee
2019-08-26  2:43 ` [PATCH 02/11] packfile: use hashmap_entry in delta_base_cache_entry Eric Wong
2019-08-26  2:43 ` [PATCH 03/11] hashmap_entry_init takes "struct hashmap_entry *" Eric Wong
2019-08-27 13:35   ` Derrick Stolee
2019-08-28 15:01     ` Johannes Schindelin
2019-08-30 19:48       ` Eric Wong
2019-09-02 13:46         ` Johannes Schindelin
2019-08-26  2:43 ` Eric Wong [this message]
2019-08-27  9:10   ` [PATCH 04/11] hashmap_entry: detect improper initialization Johannes Schindelin
2019-08-27  9:49     ` Eric Wong
2019-08-27 22:16       ` Junio C Hamano
2019-08-28 15:04         ` Johannes Schindelin
2019-08-28  9:03       ` Phillip Wood
2019-08-30 19:52         ` Eric Wong
2019-09-08  7:49   ` [RFC 04/11] coccicheck: detect hashmap_entry.hash assignment Eric Wong
2019-09-09 18:15     ` Junio C Hamano
2019-08-26  2:43 ` [PATCH 05/11] hashmap_get_next takes "const struct hashmap_entry *" Eric Wong
2019-08-26  2:43 ` [PATCH 06/11] hashmap_add takes "struct " Eric Wong
2019-08-26  2:43 ` [PATCH 07/11] hashmap_get takes "const struct " Eric Wong
2019-08-26  2:43 ` [PATCH 08/11] hashmap_remove " Eric Wong
2019-08-26  2:43 ` [PATCH 09/11] hashmap_put takes "struct " Eric Wong
2019-08-26  2:43 ` [PATCH 10/11] introduce container_of macro Eric Wong
2019-08-27 14:49   ` Derrick Stolee
2019-08-28  9:11     ` Phillip Wood
2019-08-30 19:43     ` Eric Wong
2019-08-26  2:43 ` [PATCH 11/11] hashmap_get_next returns "struct hashmap_entry *" Eric Wong
2019-08-27 14:53   ` Derrick Stolee
2019-08-30 19:36     ` Eric Wong
2019-09-24  1:03 ` [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes Eric Wong
2019-09-24  1:03   ` [PATCH v2 01/19] diff: use hashmap_entry_init on moved_entry.ent Eric Wong
2019-09-24  1:03   ` [PATCH v2 02/19] coccicheck: detect hashmap_entry.hash assignment Eric Wong
2019-09-25 12:44     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 03/19] packfile: use hashmap_entry in delta_base_cache_entry Eric Wong
2019-09-24  1:03   ` [PATCH v2 04/19] hashmap_entry_init takes "struct hashmap_entry *" Eric Wong
2019-09-25 12:48     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 05/19] hashmap_get_next takes "const struct " Eric Wong
2019-09-24  1:03   ` [PATCH v2 06/19] hashmap_add takes "struct " Eric Wong
2019-09-24  1:03   ` [PATCH v2 07/19] hashmap_get takes "const struct " Eric Wong
2019-09-25 12:52     ` Derrick Stolee
2019-09-30  9:57       ` Eric Wong
2019-09-24  1:03   ` [PATCH v2 08/19] hashmap_remove " Eric Wong
2019-09-25 12:54     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 09/19] hashmap_put takes "struct " Eric Wong
2019-09-24  1:03   ` [PATCH v2 10/19] introduce container_of macro Eric Wong
2019-09-25 13:12     ` Derrick Stolee
2019-09-30 10:39       ` Eric Wong
2019-09-24  1:03   ` [PATCH v2 11/19] hashmap_get_next returns "struct hashmap_entry *" Eric Wong
2019-09-25 13:05     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 12/19] hashmap: use *_entry APIs to wrap container_of Eric Wong
2019-09-25 13:15     ` Derrick Stolee
2019-09-24  1:03   ` [PATCH v2 13/19] hashmap_get{,_from_hash} return "struct hashmap_entry *" Eric Wong
2019-09-24  1:03   ` [PATCH v2 14/19] hashmap_cmp_fn takes hashmap_entry params Eric Wong
2019-09-24  1:03   ` [PATCH v2 15/19] hashmap: use *_entry APIs for iteration Eric Wong
2019-09-24  1:03   ` [PATCH v2 16/19] hashmap: hashmap_{put,remove} return hashmap_entry * Eric Wong
2019-09-24  1:03   ` [PATCH v2 17/19] hashmap: introduce hashmap_free_entries Eric Wong
2019-09-24  1:03   ` [PATCH v2 18/19] OFFSETOF_VAR macro to simplify hashmap iterators Eric Wong
2019-09-30 16:58     ` Junio C Hamano
2019-10-04  1:18     ` Junio C Hamano
2019-10-04  2:51       ` Eric Wong
2019-10-04  3:29         ` Junio C Hamano
2019-10-04 17:26           ` Eric Wong
2019-10-06  0:04             ` Junio C Hamano
2019-09-24  1:03   ` [PATCH v2 19/19] hashmap: remove type arg from hashmap_{get,put,remove}_entry Eric Wong
2019-09-25 12:42     ` Derrick Stolee
2019-09-25 13:30   ` [PATCH v2 00/19] hashmap bug/safety/ease-of-use fixes Derrick Stolee
2019-09-26  8:39   ` Johannes Schindelin
2019-09-30 10:01     ` Eric Wong
2019-09-26 13:48   ` Phillip Wood
2019-09-29  9:22   ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20190826024332.3403-5-e@80x24.org \
    --to=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).