[v3,07/13] strmap: add more utility functions

Commit Message

Elijah Newren Nov. 2, 2020, 6:55 p.m. UTC

From: Elijah Newren <[e-mail]>

This adds a number of additional convienence functions I want/need:
  * strmap_get_size()
  * strmap_empty()
  * strmap_remove()
  * strmap_for_each_entry()
  * strmap_get_entry()

I suspect the first four are self-explanatory.

strmap_get_entry() is similar to strmap_get() except that instead of just
returning the void* value that the string maps to, it returns the
strmap_entry that contains both the string and the void* value (or
NULL if the string isn't in the map).  This is helpful because it avoids
multiple lookups, e.g. in some cases a caller would need to call:
  * strmap_contains() to check that the map has an entry for the string
  * strmap_get() to get the void* value
  * <do some work to update the value>
  * strmap_put() to update/overwrite the value
If the void* pointer returned really is a pointer, then the last step is
unnecessary, but if the void* pointer is just cast to an integer then
strmap_put() will be needed.  In contrast, one can call strmap_get_entry()
and then:
  * check if the string was in the map by whether the pointer is NULL
  * access the value via entry->value
  * directly update entry->value
meaning that we can replace two or three hash table lookups with one.

Signed-off-by: Elijah Newren <[e-mail]>
---
 strmap.c | 20 ++++++++++++++++++++
 strmap.h | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

Comments

Jeff King Nov. 4, 2020, 8:13 p.m. UTC | #1

On Mon, Nov 02, 2020 at 06:55:07PM +0000, Elijah Newren via GitGitGadget wrote:

> +/*
> + * iterate through @map using @iter, @var is a pointer to a type strmap_entry
> + */
> +#define strmap_for_each_entry(mystrmap, iter, var)	\
> +	for (var = hashmap_iter_first_entry_offset(&(mystrmap)->map, iter, 0); \
> +		var; \
> +		var = hashmap_iter_next_entry_offset(iter, 0))
> +

I think this resolves my offset question from the last round. But I
wonder if you tried:

  #define strmap_for_each_entry(mystrmap, iter, var) \
	hashmap_for_each_entry(&(mystrmap)->map, iter, var, ent)

which is a bit more abstract and should function the same (I think; I
didn't try it).

-Peff

Elijah Newren Nov. 4, 2020, 8:24 p.m. UTC | #2

On Wed, Nov 4, 2020 at 12:13 PM Jeff King <[e-mail]> wrote:
>
> On Mon, Nov 02, 2020 at 06:55:07PM +0000, Elijah Newren via GitGitGadget wrote:
>
> > +/*
> > + * iterate through @map using @iter, @var is a pointer to a type strmap_entry
> > + */
> > +#define strmap_for_each_entry(mystrmap, iter, var)   \
> > +     for (var = hashmap_iter_first_entry_offset(&(mystrmap)->map, iter, 0); \
> > +             var; \
> > +             var = hashmap_iter_next_entry_offset(iter, 0))
> > +
>
> I think this resolves my offset question from the last round. But I
> wonder if you tried:
>
>   #define strmap_for_each_entry(mystrmap, iter, var) \
>         hashmap_for_each_entry(&(mystrmap)->map, iter, var, ent)
>
> which is a bit more abstract and should function the same (I think; I
> didn't try it).

I tried another variant or two besides what I used here, but not the
one you suggest.  Your suggestion seems obvious and nicer now that you
point it out.

Patch

diff --git a/strmap.c b/strmap.c
index 53f284eb20..829f1bc095 100644
--- a/strmap.c
+++ b/strmap.c
@@ -87,6 +87,11 @@  void *strmap_put(struct strmap *map, const char *str, void *data)
 	return old;
 }
 
+struct strmap_entry *strmap_get_entry(struct strmap *map, const char *str)
+{
+	return find_strmap_entry(map, str);
+}
+
 void *strmap_get(struct strmap *map, const char *str)
 {
 	struct strmap_entry *entry = find_strmap_entry(map, str);
@@ -97,3 +102,18 @@  int strmap_contains(struct strmap *map, const char *str)
 {
 	return find_strmap_entry(map, str) != NULL;
 }
+
+void strmap_remove(struct strmap *map, const char *str, int free_value)
+{
+	struct strmap_entry entry, *ret;
+	hashmap_entry_init(&entry.ent, strhash(str));
+	entry.key = str;
+	ret = hashmap_remove_entry(&map->map, &entry, ent, NULL);
+	if (!ret)
+		return;
+	if (free_value)
+		free(ret->value);
+	if (map->strdup_strings)
+		free((char*)ret->key);
+	free(ret);
+}
diff --git a/strmap.h b/strmap.h
index 96888c23ad..ee4307cca5 100644
--- a/strmap.h
+++ b/strmap.h
@@ -50,6 +50,12 @@  void strmap_clear(struct strmap *map, int free_values);
  */
 void *strmap_put(struct strmap *map, const char *str, void *data);
 
+/*
+ * Return the strmap_entry mapped by "str", or NULL if there is not such
+ * an item in map.
+ */
+struct strmap_entry *strmap_get_entry(struct strmap *map, const char *str);
+
 /*
  * Return the data pointer mapped by "str", or NULL if the entry does not
  * exist.
@@ -62,4 +68,34 @@  void *strmap_get(struct strmap *map, const char *str);
  */
 int strmap_contains(struct strmap *map, const char *str);
 
+/*
+ * Remove the given entry from the strmap.  If the string isn't in the
+ * strmap, the map is not altered.
+ */
+void strmap_remove(struct strmap *map, const char *str, int free_value);
+
+/*
+ * Return how many entries the strmap has.
+ */
+static inline unsigned int strmap_get_size(struct strmap *map)
+{
+	return hashmap_get_size(&map->map);
+}
+
+/*
+ * Return whether the strmap is empty.
+ */
+static inline int strmap_empty(struct strmap *map)
+{
+	return strmap_get_size(map) == 0;
+}
+
+/*
+ * iterate through @map using @iter, @var is a pointer to a type strmap_entry
+ */
+#define strmap_for_each_entry(mystrmap, iter, var)	\
+	for (var = hashmap_iter_first_entry_offset(&(mystrmap)->map, iter, 0); \
+		var; \
+		var = hashmap_iter_next_entry_offset(iter, 0))
+
 #endif /* STRMAP_H */