Re: [PATCH v2 09/10] builtin/cat-file: deduplicate logic to iterate over all objects
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 2, 2025, 11:13 UTC
- Message-ID
- <Z-0byX56V2E1pGKO@pks.im>
- In-Reply-To
- <87jz84f5a2.fsf@iotcl.com>
On Tue, Apr 01, 2025 at 02:13:57PM +0200, Toon Claes wrote:
Show 43 quoted lines
> 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