Re: [PATCH v2 06/10] packfile: packed_object_info avoids packed_to_object_type
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 26, 2024, 21:50 UTC
- Message-ID
- <xmqq7cc3gdhc.fsf@gitster.g>
- In-Reply-To
- <20240823224630.1180772-7-e@80x24.org>
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.
Show 9 quoted lines
> @@ -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.
Show 7 quoted lines
> @@ -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?
Show 7 quoted lines
> @@ -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.
Show 24 quoted lines
> @@ -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;
> }