On Tue, Oct 13, 2020 at 12:40:48AM +0000, Elijah Newren via GitGitGadget wrote:
> Although strmap could be used as a string->int map, one either had to
> allocate an int for every entry and then deallocate later, or one had to
> do a bunch of casting between (void*) and (intptr_t).
>
> Add some special functions that do the casting. Also, rename put->set
> for such wrapper functions since 'put' implied there may be some
> deallocation needed if the string was already found in the map, which
> isn't the case when we're storing an int value directly in the void*
> slot instead of using the void* slot as a pointer to data.
I think this is worth doing. That kind of casting is an implementation
detail, and it's nice for callers not to have to see it.
You might want to mention that this _could_ be done as just accessors to
strmap, but using a separate struct provides type safety against
misusing pointers as integers or vice versa.
> A note on the name: if anyone has a better name suggestion than
> strintmap, I'm happy to take it. It seems slightly unwieldy, but I have
> not been able to come up with a better name.
I still don't have a better suggestion on the name. Another convention
could be to name map types as "map_from_to". So "struct map_str_int".
But it's pretty ugly, and strmap would become "map_str_ptr" or
something. As ugly as "strintmap" is, I like it better.
> +void strintmap_incr(struct strintmap *map, const char *str, intptr_t amt)
> +{
> + struct strmap_entry *entry = find_strmap_entry(&map->map, str);
> + if (entry) {
> + intptr_t *whence = (intptr_t*)&entry->value;
> + *whence += amt;
> + }
> + else
> + strintmap_set(map, str, amt);
> +}
Incrementing a missing entry auto-vivifies it at 0. That makes perfect
sense, but might be worth noting above the function in the header file.
Though maybe it's a little weird since strintmap_get() takes a default
value. Why don't we use that here? I'd have to see how its used, but
would it make sense to set a default value when initializing the map,
rather than providing it on each call?
> +/*
> + * strintmap:
> + * A map of string -> int, typecasting the void* of strmap to an int.
Are the size and signedness of an int flexible enough for all uses?
I doubt the standard makes any promises about the relationship between
intptr_t and int, but I'd be surprised if any modern platform has an
intptr_t that isn't at least as big as an int (on most 32-bit platforms
they'll be the same, and on 64-bit ones intptr_t is strictly bigger).
Would any callers care about using the full 32-bits, though? I.e., would
they prefer casting through uintptr_t to an "unsigned int"?
-Peff
