Re: [PATCH v2 09/10] builtin/cat-file: deduplicate logic to iterate over all objects
- From
Toon Claes <toon@iotcl.com>
- Date
- Apr 1, 2025, 12:13 UTC
- Message-ID
- <87jz84f5a2.fsf@iotcl.com>
- In-Reply-To
- <20250327-pks-cat-file-object-type-filter-v2-9-4bbc7085d7c5@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 104 quoted lines
> Pull out a common function that allows us to iterate over all objects in
> a repository. Right now the logic is trivial and would only require two
> function calls, making this refactoring a bit pointless. But in the next
> commit we will iterate on this logic to make use of bitmaps, so this is
> about to become a bit more complex.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/cat-file.c | 85 ++++++++++++++++++++++++++++++------------------------
> 1 file changed, 48 insertions(+), 37 deletions(-)
>
> 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
> @@ -622,25 +622,18 @@ static int batch_object_cb(const struct object_id *oid, void *vdata)
> return 0;
> }
>
> -static int collect_loose_object(const struct object_id *oid,
> - const char *path UNUSED,
> - void *data)
> -{
> - oid_array_append(data, oid);
> - return 0;
> -}
> -
> -static int collect_packed_object(const struct object_id *oid,
> - struct packed_git *pack UNUSED,
> - uint32_t pos UNUSED,
> - void *data)
> +static int collect_object(const struct object_id *oid,
> + struct packed_git *pack UNUSED,
> + off_t offset UNUSED,
> + void *data)
> {
> oid_array_append(data, oid);
> return 0;
> }
>
> static int batch_unordered_object(const struct object_id *oid,
> - struct packed_git *pack, off_t offset,
> + struct packed_git *pack,
> + off_t offset,
> void *vdata)
> {
> struct object_cb_data *data = vdata;
> @@ -654,23 +647,6 @@ static int batch_unordered_object(const struct object_id *oid,
> return 0;
> }
>
> -static int batch_unordered_loose(const struct object_id *oid,
> - const char *path UNUSED,
> - void *data)
> -{
> - return batch_unordered_object(oid, NULL, 0, data);
> -}
> -
> -static int batch_unordered_packed(const struct object_id *oid,
> - struct packed_git *pack,
> - uint32_t pos,
> - void *data)
> -{
> - return batch_unordered_object(oid, pack,
> - nth_packed_object_offset(pack, pos),
> - data);
> -}
> -
> typedef void (*parse_cmd_fn_t)(struct batch_options *, const char *,
> struct strbuf *, struct expand_data *);
>
> @@ -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.
-- Toon