Jeff King <peff@xxxxxxxx> writes:
> @@ -460,6 +462,8 @@ int prepare_midx_pack(struct repository *r, struct multi_pack_index *m,
>
> if (m->packs[pack_int_id])
> return 0;
> + if (m->pack_errs[pack_int_id])
> + return 1;
>
> strbuf_addf(&pack_name, "%s/pack/%s", m->object_dir,
> m->pack_names[pack_int_id]);
> @@ -482,8 +486,10 @@ int prepare_midx_pack(struct repository *r, struct multi_pack_index *m,
> strbuf_release(&pack_name);
> strbuf_release(&key);
>
> - if (!p)
> + if (!p) {
> + m->pack_errs[pack_int_id] = 1;
> return 1;
> + }
>
> p->multi_pack_index = 1;
> m->packs[pack_int_id] = p;
>
> You could even lazy-malloc the extra array if you wanted to optimize the
> common no-errors case, but I'm not sure it's a big deal.
So the idea is to leave m->packs[] for unused pack NULL instead of
magic value, so that users of that array do not need to check?
I think that is a lot safer than the magic "we know this fails"
value that existing callers that have long trusted that a non-NULL
.packs[] element is a valid pack.
By the way, I suspect I am not reading the code correctly, but I am
not sure what fill_midx_entry() does with a failed case.
midx_for_object(&m, pos);
pack_int_id = nth_midxed_pack_int_id(m, pos);
if (prepare_midx_pack(r, m, pack_int_id))
return 0;
With or without cached failure, this should return 0 when and only
when m->packs[pack_int_id] is a usable pack. But what about the
access on the next line?
p = m->packs[pack_int_id - m->num_packs_in_base];
Do we have any guarantee that we called prepare_midx_pack() for
the pack at (pack_int_id - m->num_packs_in_base)th slot? Can p
be NULL here? And with the magic "we know this fails" value, can p
be that magic value?
/*
* We are about to tell the caller where they can locate the
* requested object. We better make sure the packfile is
* still here and can be accessed before supplying that
* answer, as it may have been deleted since the MIDX was
* loaded!
*/
if (!is_pack_valid(p))
return 0;
That value is passed to is_pack_valid(), which I do not think even
takes NULL.
Puzzled.
