Re: [PATCH olang 2/2] utils: create hash map data structure

[Johnny Richard <johnny@johnnyrichard.com>]

On Tue, Feb 27, 2024 at 02:26:17PM -0300, Carlos Maniero wrote:
> diff --git a/src/arena.h b/src/arena.h
> index 157165c..230eb5b 100644
> --- a/src/arena.h
> +++ b/src/arena.h
> @@ -38,4 +38,7 @@ arena_release(arena_t *arena);
>  void
>  arena_free(arena_t *arena);
>  
> +char *
> +arena_strdup(arena_t *arena, const char *s);

I would move it to arena if the function wasn't cstr specific. I mean,
a memcpy would be a better fit since it's agnostic / generic.

> +
>  #endif
> diff --git a/src/map.c b/src/map.c
> index 532ba3b..849130e 100644
> --- a/src/map.c
> +++ b/src/map.c
> @@ -31,8 +31,8 @@ u32_fnv1a_hash(const char *s);
>  static void
>  map_init(map_t *map);
>  
> -static char *
> -__strdup(const char *s, arena_t *arena);
> +static uint32_t
> +map_index(map_t *map, uint32_t hash);
>  
>  map_t *
>  map_new(arena_t *arena)
> @@ -78,24 +78,27 @@ map_put(map_t *map, char *key, void *value)
>  {
>      assert(map && key);
>      uint32_t hash = u32_fnv1a_hash(key);
> -    map_entry_t *entry = &(map->entries[hash & (map->capacity - 1)]);
> +    map_entry_t *entry = map->entries + map_index(map, hash);
>  
> -    while (entry != NULL) {
> +    if (entry == NULL) {
> +        *entry = (map_entry_t){ .key = arena_strdup(map->arena, key), .hash = hash, .value = value, .next = NULL };

It can cause a segfault, perhaps we should keep the 'entry->key == NULL'
since the entries are filled up with zeros (0).

> +static uint32_t
> +map_index(map_t *map, uint32_t hash)

Nice change, what do you think about rename it to **map_get_index**?

> diff --git a/tests/unit/map_test.c b/tests/unit/map_test.c
> index 3eb9acd..fab4c69 100644
> --- a/tests/unit/map_test.c
> +++ b/tests/unit/map_test.c
> @@ -52,7 +52,10 @@ test_map_put_and_get(const MunitParameter params[], void *user_data_or_fixture)
>      assert_int(*((int *)map_get(map, "n1")), ==, n1);
>      assert_int(*((int *)map_get(map, "n2")), ==, n2);
>  
> +    map_put(map, "n1", (void *)&n2);
> +
>      arena_free(&arena);
> +    assert_int(*((int *)map_get(map, "n1")), ==, n2);

Wired this work after free arena.