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

On Thu, Aug 27, 2026 at 06:00:37AM -0400, Karthik Nayak wrote:

Patrick Steinhardt [off-list ref] writes:

quoted

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'.

I actually didn't have the "almost" initially but added it later, but
there isn't really much of a reason why specifically I did so. I'll drop
it again.

quoted

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 892c5661d9..3c4127f4d8 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c

@@ -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.

quoted

-		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.

Yeah. We could just drop this, but then it'd feel a tiny bit weird as we
call the function without checking its return value at all. So I decided
to just keep this as-is.

Patrick