On 8/20/2018 5:23 PM, Stefan Beller wrote:
On Mon, Aug 20, 2018 at 9:52 AM Derrick Stolee <dstolee@xxxxxxxxxxxxx> wrote:
When an object fails to decompress from a pack-file, we mark the object
as 'bad' so we can retry with a different copy of the object (if such a
copy exists).
Before now, the multi-pack-index did not update the bad objects list for
the pack-files it contains, and we did not check the bad objects list
when reading an object. Now, do both.
This sounds like a bug fix unlike patches 1 & 2 that sound like
feature work(2) or making code more readable(1).
(After studying the code, this doesn't sound like a bug fix any more,
but a safety thing)
It is a safety thing. One codepath that needs this includes this comment:
/*
* We're probably in deep shit, but let's try to fetch
* the required base anyway from another pack or loose.
* This is costly but should happen only in the presence
* of a corrupted pack, and is better than failing outright.
*/
This goes back to 8eca0b47: implement some resilience against pack
corruptions. The logic is tested in t5303-pack-corruption-resilience.sh,
with the test title '... and a redundant pack allows for full recovery too'.
Is it worth having this on a separate track coming in
faster than the rest of this series?
ds/multi-pack-index is cooking in 'next' until after 2.19.0, so I'm not
sure it's worth splitting things up at this point.
Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
---
midx.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/midx.c b/midx.c
index 6824acf5f8..7fa75a37a3 100644
--- a/midx.c
+++ b/midx.c
@@ -280,6 +280,16 @@ static int nth_midxed_pack_entry(struct multi_pack_index *m, struct pack_entry *
if (!is_pack_valid(p))
return 0;
+ if (p->num_bad_objects) {
+ uint32_t i;
Is there a reason that i needs to be if 32 bits?
Would size_t (for iterating) or 'int' (as a default
like in many for loops) be ok, too?
i is bounded by p->num_bad_objects, which is also uint32_t.
Thanks,
-Stolee
