[PATCH 4/5] strmap: add strdup_strings option

From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Elijah Newren <newren@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH 4/5] strmap: add strdup_strings option
Date: Fri, 21 Aug 2020 18:52:28 +0000	[thread overview]
Message-ID: <b3095d97d8ee9d6576292731cc100492e7c64f13.1598035949.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.835.git.git.1598035949.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

Just as it is sometimes useful for string_list to duplicate and take
ownership of memory management of the strings it contains, the same is
sometimes true for strmaps as well.  Add the same flag from string_list
to strmap.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 strmap.c | 23 ++++++++++++++++-------
 strmap.h |  9 +++++----
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/strmap.c b/strmap.c
index a4bfffcd8b..03eb6af45d 100644
--- a/strmap.c
+++ b/strmap.c
@@ -22,9 +22,10 @@ static struct str_entry *find_str_entry(struct strmap *map,
 	return hashmap_get_entry(&map->map, &entry, ent, NULL);
 }
 
-void strmap_init(struct strmap *map)
+void strmap_init(struct strmap *map, int strdup_strings)
 {
 	hashmap_init(&map->map, cmp_str_entry, NULL, 0);
+	map->strdup_strings = strdup_strings;
 }
 
 void strmap_free(struct strmap *map, int free_util)
@@ -35,9 +36,10 @@ void strmap_free(struct strmap *map, int free_util)
 	if (!map)
 		return;
 
-	if (free_util) {
+	if (map->strdup_strings || free_util) {
 		hashmap_for_each_entry(&map->map, &iter, e, ent) {
-			free(e->item.string);
+			if (map->strdup_strings)
+				free(e->item.string);
 			if (free_util)
 				free(e->item.util);
 		}
@@ -48,12 +50,11 @@ void strmap_free(struct strmap *map, int free_util)
 void strmap_clear(struct strmap *map, int free_util)
 {
 	strmap_free(map, free_util);
-	strmap_init(map);
+	strmap_init(map, map->strdup_strings);
 }
 
 /*
- * Insert "str" into the map, pointing to "data". A copy of "str" is made, so
- * it does not need to persist after the this function is called.
+ * Insert "str" into the map, pointing to "data".
  *
  * If an entry for "str" already exists, its data pointer is overwritten, and
  * the original data pointer returned. Otherwise, returns NULL.
@@ -69,7 +70,13 @@ void *strmap_put(struct strmap *map, const char *str, void *data)
 	} else {
 		entry = xmalloc(sizeof(*entry));
 		hashmap_entry_init(&entry->ent, strhash(str));
-		entry->item.string = strdup(str);
+		/*
+		 * We won't modify entry->item.string so it really should be
+		 * const, but changing string_list_item to use a const char *
+		 * is a bit too big of a change at this point.
+		 */
+		entry->item.string =
+			map->strdup_strings ? xstrdup(str) : (char *)str;
 		entry->item.util = data;
 		hashmap_add(&map->map, &entry->ent);
 	}
@@ -100,6 +107,8 @@ void strmap_remove(struct strmap *map, const char *str, int free_util)
 	hashmap_entry_init(&entry.ent, strhash(str));
 	entry.item.string = (char *)str;
 	ret = hashmap_remove_entry(&map->map, &entry, ent, NULL);
+	if (map->strdup_strings)
+		free(ret->item.string);
 	if (ret && free_util)
 		free(ret->item.util);
 	free(ret);
diff --git a/strmap.h b/strmap.h
index 45d0a4f714..28a98c5a4b 100644
--- a/strmap.h
+++ b/strmap.h
@@ -6,6 +6,7 @@
 
 struct strmap {
 	struct hashmap map;
+	unsigned int strdup_strings:1;
 };
 
 struct str_entry {
@@ -14,9 +15,10 @@ struct str_entry {
 };
 
 /*
- * Initialize an empty strmap
+ * Initialize the members of the strmap, set `strdup_strings`
+ * member according to the value of the second parameter.
  */
-void strmap_init(struct strmap *map);
+void strmap_init(struct strmap *map, int strdup_strings);
 
 /*
  * Remove all entries from the map, releasing any allocated resources.
@@ -29,8 +31,7 @@ void strmap_free(struct strmap *map, int free_values);
 void strmap_clear(struct strmap *map, int free_values);
 
 /*
- * Insert "str" into the map, pointing to "data". A copy of "str" is made, so
- * it does not need to persist after the this function is called.
+ * Insert "str" into the map, pointing to "data".
  *
  * If an entry for "str" already exists, its data pointer is overwritten, and
  * the original data pointer returned. Otherwise, returns NULL.
-- 
gitgitgadget

