Re: [PATCH 01/10] builtin/fsck: use `fsck_obj_buffer()` when checking loose objects

From: Karthik Nayak <karthik.188@gmail.com>
Date: Aug 27, 2026, 10:00 UTC
Message-ID: <CAOLa=ZQyGk65qhF2uRV6Q48c_F948XuTWOdHOTwenfGestLxWw@mail.gmail.com>
In-Reply-To: <20260825-pks-odb-source-fsck-v1-1-b756de0bf24f@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> When checking loose objects we manually parse the object buffer we have
> read from the on-disk file, mark the object and then call `fsck_obj()`.
> Almost the exact same steps are also performed by `fsck_obj_buffer()`.
>

I was wondering what the difference was, there seems to be none, nit: perhaps we can drop 'Almost'.

Show 32 quoted lines

> Stop open-coding this logic and call `fsck_obj_buffer()` instead.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/fsck.c | 17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 892c5661d9..3c4127f4d8 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -722,7 +722,6 @@ static int fsck_loose(const struct object_id *oid, const char *path,
>  		      void *cb_data)
>  {
>  	struct for_each_loose_cb *data = cb_data;
> -	struct object *obj;
>  	enum object_type type = OBJ_NONE;
>  	size_t size;
>  	void *contents = NULL;
> @@ -751,21 +750,7 @@ static int fsck_loose(const struct object_id *oid, const char *path,
>  	if (!contents && type != OBJ_BLOB)
>  		BUG("read_loose_object streamed a non-blob");
>
> -	obj = parse_object_buffer(data->repo, oid, type, size,
> -				  contents, &eaten);
> -
> -	if (!obj) {
> -		errors_found |= ERROR_OBJECT;
> -		error(_("%s: object could not be parsed: %s"),
> -		      oid_to_hex(oid), path);
> -		if (!eaten)
> -			free(contents);

This is now moved to the bottom below fsck_obj_buffer() call. So that's okay.

Show 9 quoted lines

> -		return 0; /* keep checking other objects */
> -	}
> -
> -	obj->flags &= ~(REACHABLE | SEEN);
> -	obj->flags |= HAS_OBJ;
> -	if (fsck_obj(data->repo, obj, contents, size))
> +	if (fsck_obj_buffer(oid, type, size, contents, &eaten, data->repo))
>  		errors_found |= ERROR_OBJECT;
>

I see `fsck_obj_buffer()` also sets adds the `ERROR_OBJECT` flag, but that's okay.

>  	if (!eaten)
>
> --
> 2.55.0.822.g20453c30eb.dirty