From: Patrick Steinhardt Date: Mon, 31 Aug 2026 06:00:24 GMT Subject: Re: [PATCH 01/10] builtin/fsck: use `fsck_obj_buffer()` when checking loose objects Message-ID: In-Reply-To: On Thu, Aug 27, 2026 at 06:00:37AM -0400, Karthik Nayak wrote: > Patrick Steinhardt 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'. 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. > > 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. > > > - 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