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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]

On Mon, Aug 02, 2021 at 02:55:09PM +0200, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Aug 02 2021, Patrick Steinhardt wrote:
[snip]
> > 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]