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

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 5, 2021, 10:12 UTC
Message-ID: <YQu5pyjcaye3P0N3@ncase>
In-Reply-To: <87v94onguw.fsf@evledraar.gmail.com>

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

> On Mon, Aug 02 2021, Patrick Steinhardt wrote:

[snip]

Show 21 quoted lines

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