From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: mail-a.sr.ht; dkim=pass header.d=johnnyrichard.com header.i=@johnnyrichard.com
Received: from out-176.mta0.migadu.com (out-176.mta0.migadu.com [91.218.175.176])
	by mail-a.sr.ht (Postfix) with ESMTPS id 1F3C6200F9
	for <~johnnyrichard/olang-devel@lists.sr.ht>; Tue, 27 Feb 2024 18:31:06 +0000 (UTC)
Date: Tue, 27 Feb 2024 20:30:56 +0100
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=johnnyrichard.com;
	s=key1; t=1709058664;
	h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
	 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
	 in-reply-to:in-reply-to:references:references;
	bh=1MpAYIwZlASeEC4wU4PwINNT8qKW/C+385XLk5k4EdM=;
	b=Kf32JoIrIKh9f9fQQPBAOIoK3TiZhYvhvlxlDUeBh8JboT9S3/MJ4GYIkip/Pt9e0jGtl5
	QB1MjrwcostWoO5C6gEsNep2ye7iCqbnBb8KhsGAnTZdZ0zrwhQldIVy9aJUclyRbffHUy
	h6XNX5s3z9s84FWCDDLHEdifTZCp2H/uqg29YAkWIz6bRoXAfy2TTFhqgX6emcuOYzZS8I
	UIe4j8ARIax/9mMWFtml4oHhg+Wpt7proE0vaypqN4N6vgBCfT+vg+5qmET59baiqr7nsR
	ckrpnQB2Z8hvKKcMi8xvL/ykCsFhIrBAwK3FklxKrn6Lc+mhz27IE/T3Qwb7XA==
X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers.
From: Johnny Richard <johnny@johnnyrichard.com>
To: Carlos Maniero <carlos@maniero.me>
Cc: ~johnnyrichard/olang-devel@lists.sr.ht
Subject: Re: [PATCH olang 2/2] utils: create hash map data structure
Message-ID: <4vjiv6ckgbqpm7ywtwsv5fjh4n3fn356ugotr6xv7nm35y2ivz@wkqdfiuqfigv>
References: <20240221222226.67254-1-johnny@johnnyrichard.com>
 <20240221222226.67254-3-johnny@johnnyrichard.com>
 <CZG1CU5AIUW0.3VVCBS15UFO08@maniero.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CZG1CU5AIUW0.3VVCBS15UFO08@maniero.me>
X-Migadu-Flow: FLOW_OUT
X-TUID: qpfJ9hkz0X18

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.