Nicolas Pitre <nico@xxxxxxx> writes: > It is those with semantic meaning (e.g. object doesn't exist) which > should be audited, especially if used in the context of repository > modification, which pretty much limits it to the test case I produced. I've been wondering if we should make the change 8eca0b4 (implement some resilience against pack corruptions, 2008-06-23) less aggressive. It makes loose objects and data from other packs to be used as fallback where we used to just punt, which is a genuine improvement for "salvaging" mode of operation, but at the same time, it now forbids the callers to expect that the objects they learned to exist from has_sha1_file() or nth_packed_object_sha1() should never result NULL return value from read_sha1_file(). It may make it safe again to fail if you cannot salvage using fallback method after all. Something like the attached. This is unrelated to the issue at hand, but I also notice that there are few callsites outside sha1_file.c that bypasses cache_or_unpack_entry() and call unpack_entry() directly. I wonder if they should be using the cached version, making unpack_entry() static... sha1_file.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 2df78b5..55aa361 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1649,7 +1649,7 @@ static void *unpack_delta_entry(struct packed_git *p, mark_bad_packed_object(p, base_sha1); base = read_sha1_file(base_sha1, type, &base_size); if (!base) - return NULL; + exit(129); } delta_data = unpack_compressed_entry(p, w_curs, curpos, delta_size); @@ -1946,6 +1946,8 @@ static void *read_packed_sha1(const unsigned char *sha1, sha1_to_hex(sha1), (uintmax_t)e.offset, e.p->pack_name); mark_bad_packed_object(e.p, sha1); data = read_sha1_file(sha1, type, size); + if (!data) + exit(129); } return data; } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
