Re: [PATCH v2 06/10] packfile: packed_object_info avoids packed_to_object_type

[Junio C Hamano <gitster@pobox.com>]

Eric Wong <e@80x24.org> writes:

> Subject: Re: [PATCH v2 06/10] packfile: packed_object_info avoids packed_to_object_type

I was confused if "avoids" is talking about the status quo, avoiding
is the bad thing, and this patch is about fixing that so that it
would not avoid.  That is not what this does.

    Subject: packfile: skip packed_to_object_type call in packed_object_info

or something, perhaps?

> For entries in the delta base cache, packed_to_object_type calls
> can be omitted.

"... because an entry in delta base cache knows what the final type
of the object is"?

I had "what the code should do in the error cases" question in a few
places.  Please describe (and justify if needed) the choice of
behaviour in the proposed log message.

> @@ -1538,7 +1538,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,
>  	ent = get_delta_base_cache_entry(p, obj_offset);
>  	if (ent) {
>  		oi->whence = OI_DBCACHED;
> -		type = ent->type;
> +		final_type = type = ent->type;
>  		if (oi->sizep)
>  			*oi->sizep = ent->size;
>  		if (oi->contentp) {

OK.

> @@ -1556,6 +1556,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,
>  	} else if (oi->contentp && !oi->content_limit) {
>  		*oi->contentp = unpack_entry(r, p, obj_offset, &type,
>  						oi->sizep);
> +		final_type = type;
>  		if (!*oi->contentp)
>  			type = OBJ_BAD;

Do we want to still yield the "type" not "OBJ_BAD" in final_type in
this case?

> @@ -1585,6 +1586,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,
>  			if (oi->sizep && *oi->sizep <= oi->content_limit) {
>  				*oi->contentp = unpack_entry(r, p, obj_offset,
>  							&type, oi->sizep);
> +				final_type = type;
>  				if (!*oi->contentp)
>  					type = OBJ_BAD;

Ditto.

> @@ -1606,17 +1608,17 @@ int packed_object_info(struct repository *r, struct packed_git *p,
>  	}
>  
>  	if (oi->typep || oi->type_name) {
> -		enum object_type ptot;
> -		ptot = packed_to_object_type(r, p, obj_offset,
> -					     type, &w_curs, curpos);
> +		if (final_type < 0)
> +			final_type = packed_to_object_type(r, p, obj_offset,
> +						     type, &w_curs, curpos);
>  		if (oi->typep)
> -			*oi->typep = ptot;
> +			*oi->typep = final_type;
>  		if (oi->type_name) {
> -			const char *tn = type_name(ptot);
> +			const char *tn = type_name(final_type);
>  			if (tn)
>  				strbuf_addstr(oi->type_name, tn);
>  		}
> -		if (ptot < 0) {
> +		if (final_type < 0) {
>  			type = OBJ_BAD;
>  			goto out;
>  		}