Re: [PATCH v3 06/14] packfile: introduce function to iterate through objects

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 23, 2026, 00:06 UTC
Message-ID: <aXK7cSJW2syew89a@nand.local>
In-Reply-To: <20260121-pks-odb-for-each-object-v3-6-12c4dfd24227@pks.im>

On Wed, Jan 21, 2026 at 01:50:22PM +0100, Patrick Steinhardt wrote:

> Introduce a new function `packfile_store_for_each_object()`. This
> function is the equivalent to `odb_source_loose_for_each_object()` in

s/to/of/ ?

> that it:
>
>   - Works on a single packfile store and thus per object source.

s/thus per/thus/ ?

Show 23 quoted lines

> diff --git a/packfile.c b/packfile.c
> index d15a2ce12b..cd45c6f21c 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -2360,6 +2360,54 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,
>  	return ret ? ret : pack_errors;
>  }
>
> +struct packfile_store_for_each_object_wrapper_data {
> +	struct packfile_store *store;
> +	struct object_info *oi;
> +	odb_for_each_object_cb cb;
> +	void *cb_data;
> +};
> +
> +static int packfile_store_for_each_object_wrapper(const struct object_id *oid,
> +						  struct packed_git *pack,
> +						  uint32_t index_pos,
> +						  void *cb_data)
> +{
> +	struct packfile_store_for_each_object_wrapper_data *data = cb_data;
> +
> +	if (data->oi) {

Interesting. Is it the case that if the caller provides a non-NULL pointer to an object_info struct, that we will reuse the request portion for all iterated objects, updating the response portion as we go along?

If so, I am a little uneasy about the potential for us to mix portions of the response from an earlier object with a later one. Skimming packed_object_info(), I don't think that we are in any immediate danger since it overwrites all fields in the response section. But that feels somewhat fragile to me, say, if packed_object_info() were to at some point conditionally assign a field.

I wonder if we should split the request/response sections of object_info into their own object_info_req and object_info_resp structs. If we did that, then we could invert the pattern for providing the response, filling it out ourselves and then passing a pointer to it back to the caller via the callback function.

TBH, I wonder whether we should push this onto the caller entirely. If they need to make an object_info request for each object, is there any cost to having them do that explicitly themselves?

Thanks, Taylor