Re: [PATCH v2 09/10] builtin/cat-file: deduplicate logic to iterate over all objects

[Patrick Steinhardt <ps@pks.im>]

On Tue, Apr 01, 2025 at 02:13:57PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 430320adfe9..6f5dbc821a2 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -803,6 +779,45 @@ static void batch_objects_command(struct batch_options *opt,
> >  
> >  #define DEFAULT_FORMAT "%(objectname) %(objecttype) %(objectsize)"
> >  
> > +typedef int (*for_each_object_fn)(const struct object_id *oid, struct packed_git *pack,
> > +				  off_t offset, void *data);
> > +
> > +struct for_each_object_payload {
> > +	for_each_object_fn callback;
> > +	void *payload;
> > +};
> > +
> > +static int batch_one_object_loose(const struct object_id *oid,
> > +				  const char *path UNUSED,
> > +				  void *_payload)
> > +{
> > +	struct for_each_object_payload *payload = _payload;
> > +	return payload->callback(oid, NULL, 0, payload->payload);
> > +}
> > +
> > +static int batch_one_object_packed(const struct object_id *oid,
> > +				   struct packed_git *pack,
> > +				   uint32_t pos,
> > +				   void *_payload)
> > +{
> > +	struct for_each_object_payload *payload = _payload;
> > +	return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
> > +				 payload->payload);
> > +}
> > +
> > +static void batch_each_object(for_each_object_fn callback,
> > +			      unsigned flags,
> > +			      void *_payload)
> 
> Why is this `_payload` typeless? I see it only getting passed in
> `struct object_cb_data`, is there a reason to hide this type? With
> payload being wrapped in payload I think it's beneficial to keep type
> info where possible.

Because the payload gets forwarded to the callback, and that callback
accepts arbitrary types. You can already see this now: we call the
function once with a `struct object_cb_data` pointer, and once with a
`struct oid_array` pointer.

Patrick