From: Justin Tobler <jltobler@gmail.com>
Date: Thu, 15 Jan 2026 21:24:25 GMT
Subject: Re: [PATCH 08/14] builtin/fsck: refactor to use `odb_for_each_object()`
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:
> 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. :)

>  	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