Re: [PATCH/RFC v3 02/12] pack-objects: turn type and in_pack_type to bitfields
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 9, 2018, 22:54 UTC
- Message-ID
- <xmqqk1uk5076.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180308114232.10508-3-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 13 quoted lines
> @@ -1570,7 +1576,7 @@ static void drop_reused_delta(struct object_entry *entry)
> entry->depth = 0;
>
> oi.sizep = &entry->size;
> - oi.typep = &entry->type;
> + oi.typep = &type;
> if (packed_object_info(entry->in_pack, entry->in_pack_offset, &oi) < 0) {
> /*
> * We failed to get the info from this pack for some reason;
> @@ -1580,6 +1586,10 @@ static void drop_reused_delta(struct object_entry *entry)
> */
> entry->type = sha1_object_info(entry->idx.oid.hash,
> &entry->size);The comment immediately before this pre-context reads as such:
/* * We failed to get the info from this pack for some reason; * fall back to sha1_object_info, which may find another copy. * And if that fails, the error will be recorded in entry->type * and dealt with in prepare_pack(). */
The rest of the code relies on the ability of entry->type to record the error by storing an invalid (negative) type; otherwise, it cannot detect an error where (1) the entry in _this_ pack was corrupt, and (2) we wished to find another copy of the object elsewhere (which would overwrite the negative entry->type we assign here), but we didn't find any.
How should we propagate the error we found here down the control flow in this new code?
> + } else {
> + if (type < 0)
> + die("BUG: invalid type %d", type);
> + entry->type = type;The BUG() on this side is sensible, as packed_object_info() shouldn't report success when it stored negative result in *oi.typep anyway.
> unsigned char in_pack_header_size; > + unsigned type:TYPE_BITS; > + unsigned in_pack_type:TYPE_BITS; /* could be delta */