Re: [PATCH v3 3/4] revision: avoid loading object headers multiple times

On Mon, Aug 02, 2021 at 02:55:09PM +0200, Ævar Arnfjörð Bjarmason wrote:

On Mon, Aug 02 2021, Patrick Steinhardt wrote:

[snip]

quoted

diff --git a/revision.c b/revision.c
index f06a5d63a3..671b6d6513 100644
--- a/revision.c
+++ b/revision.c

@@ -359,14 +359,22 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
 				    const struct object_id *oid,
 				    unsigned int flags)
 {
-	struct object *object;
+	struct object *object = lookup_unknown_object(revs->repo, oid);
+
+	if (object->type == OBJ_NONE) {
+		int type = oid_object_info(revs->repo, oid, NULL);
+		if (type < 0 || !object_as_type(object, type, 1)) {

Let's s/int type/enum object_type, personally I think we should never do
"type < 0" either, and check OBJ_BAD explicitly, but I've seemingly lost
that discussion on-list before.

But I think the consensus is that we should not do !type, but rather
type == OBJ_NONE.

`oid_object_info()` does return an `int` though, so it feels kind of
weird to me to stuff it into an enum right away. Furthermore, while
`OBJ_BAD` does map to `-1`, the documentation of `oid_object_info()`
currently asks for what I'm doing: """returns enum object_type or
negative""".

Patrick