Re: [PATCH 08/14] builtin/fsck: refactor to use `odb_for_each_object()`
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Jan 15, 2026, 21:24 UTC
- Message-ID
- <aWlaA2cwDS39pvRx@denethor>
- In-Reply-To
- <20260115-pks-odb-for-each-object-v1-8-5418a91d5d99@pks.im>
On 26/01/15 12:04PM, Patrick Steinhardt wrote:
Show 56 quoted lines
> In git-fsck(1) we have two callsites where we iterate over all objects
> via `for_each_loose_object()` and `for_each_packed_object()`. Both of
> these are trivially convertible with `odb_for_each_object()`.
>
> Refactor these callsites accordingly.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/fsck.c | 57 ++++++++++++---------------------------------------------
> 1 file changed, 12 insertions(+), 45 deletions(-)
>
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 4979bc795e..96107695ae 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -218,15 +218,17 @@ static int mark_used(struct object *obj, enum object_type type UNUSED,
> return 0;
> }
>
> -static void mark_unreachable_referents(const struct object_id *oid)
> +static int mark_unreachable_referents(const struct object_id *oid,
> + struct object_info *io UNUSED,
> + void *data UNUSED)
> {
> struct fsck_options options = FSCK_OPTIONS_DEFAULT;
> struct object *obj = lookup_object(the_repository, oid);
>
> if (!obj || !(obj->flags & HAS_OBJ))
> - return; /* not part of our original set */
> + return 0; /* not part of our original set */
> if (obj->flags & REACHABLE)
> - return; /* reachable objects already traversed */
> + return 0; /* reachable objects already traversed */
>
> /*
> * Avoid passing OBJ_NONE to fsck_walk, which will parse the object
> @@ -243,22 +245,7 @@ static void mark_unreachable_referents(const struct object_id *oid)
> fsck_walk(obj, NULL, &options);
> if (obj->type == OBJ_TREE)
> free_tree_buffer((struct tree *)obj);
> -}
>
> -static int mark_loose_unreachable_referents(const struct object_id *oid,
> - const char *path UNUSED,
> - void *data UNUSED)
> -{
> - mark_unreachable_referents(oid);
> - return 0;
> -}
> -
> -static int mark_packed_unreachable_referents(const struct object_id *oid,
> - struct packed_git *pack UNUSED,
> - uint32_t pos UNUSED,
> - void *data UNUSED)
> -{
> - mark_unreachable_referents(oid);Ah ok, now that object iteration is unified, we don't need the two separate callbacks. Makes sense. :)
Show 15 quoted lines
> return 0; > } > > @@ -394,12 +381,8 @@ static void check_connectivity(void) > * and ignore any that weren't present in our earlier > * traversal. > */ > - for_each_loose_object(the_repository->objects, > - mark_loose_unreachable_referents, NULL, 0); > - for_each_packed_object(the_repository, > - mark_packed_unreachable_referents, > - NULL, > - 0); > + odb_for_each_object(the_repository->objects, NULL, > + mark_unreachable_referents, NULL, 0);
Nice! Now we no longer have to explicitly handle the various object backends while iterating. This patch looks good.
-Justin