Nguyễn Thái Ngọc Duy  <pclo...@gmail.com> writes:

> @@ -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 */

Reply via email to