From: Justin Tobler <jltobler@gmail.com>
Date: Wed, 17 Sep 2025 22:15:44 GMT
Subject: Re: [PATCH v5 04/15] odb: move packfile map into `struct packfile_store`
Message-ID: <54aadwxqaxm5sewgxg6aegfrvtqrdla6mxbhkrdbbufgqpb3sd@ymwtrihoj4kb>
In-Reply-To: <20250915-b4-pks-packfiles-store-v5-4-d6340350934f@pks.im>

On 25/09/15 10:54AM, Patrick Steinhardt wrote:
> The object database tracks a map of packfiles by their respective paths,
> which is used to figure out whether a given packfile has already been
> loaded. With the introduction of the `struct packfile_store` we have a
> better place to host this list though.

Makes sense.

> Move the map accordingly.
> 
> `pack_map_entry_cmp()` isn't used anywhere but in "packfile.c" anymore
> after this change, so we convert it to a static function, as well. Note
> that we also drop the `inline` hint: the function is used as a callback
> function exclusively, and callbacks cannot be inlined.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
> diff --git a/midx.c b/midx.c
> index 7726c13d7e..e96970efbf 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -460,7 +460,7 @@ int prepare_midx_pack(struct multi_pack_index *m,
>  	strbuf_addbuf(&key, &pack_name);
>  	strbuf_strip_suffix(&key, ".idx");
>  	strbuf_addstr(&key, ".pack");
> -	p = hashmap_get_entry_from_hash(&r->objects->pack_map,
> +	p = hashmap_get_entry_from_hash(&r->objects->packfiles->map,
>  					strhash(key.buf), key.buf,
>  					struct packed_git, packmap_ent);

In `struct object_database`, the comment above the defined `struct
packfile_store *packfiles` says it "should only be accessed directly by
packfile.c". Now that the packfile map has been moved into `struct
packfile_store`, it looks like "midx.c" now reaches into this structure.

Is this something we should consider?

-Justin