Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
fsck is a tool that error() is more preferred than die(), but many
more preferred without justifying why it is more preferred is
not quite a justification, is it? Also, an object failing to load
in-core is not a missing object, so if your aim is to let fsck
diagnose a too-large-to-load object as missing and let it continue,
I do not know if it is more preferred in the first place. Adding
a too large--cannot check bin of objects may be needed for it to
be useful. Also, we might need to give at the end oh by the way,
because we couldn't read some objects to even determine its type,
the unreachable report from this fsck run is totally useless.
The log message tries to justify that this may be a good thing for
fsck, but the patch actually tries to change the behaviour of all
code paths that try to load an object in-core without considering
the ramifications of such a change. I _think_ all callers should be
prepared to receive NULL when we encounter a corrupt object (and
otherwise we should fix them), but it is unclear how much audit of
the callers (if any) was done to prepare this change.
Not very excited X-.
functions embed die() inside beyond fsck's control.
unpack_compressed_entry()'s using xmallocz is such a function,
triggered from verify_packfile() - unpack_entry(). Make it use
xmallocz_gentle() instead.
Noticed-by: Dale R. Worley wor...@alum.mit.edu
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
sha1_file.c | 4 +++-
t/t1050-large.sh | 6 ++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index 34d527f..eb69c78 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1925,7 +1925,9 @@ static void *unpack_compressed_entry(struct packed_git
*p,
git_zstream stream;
unsigned char *buffer, *in;
- buffer = xmallocz(size);
+ buffer = xmallocz_gentle(size);
+ if (!buffer)
+ return NULL;
memset(stream, 0, sizeof(stream));
stream.next_out = buffer;
stream.avail_out = size + 1;
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index aea4936..5642f84 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '
git archive --format=zip HEAD /dev/null
'
+test_expect_success 'fsck' '
+ test_must_fail git fsck 2err
+ n=$(grep error: attempting to allocate .* over limit err | wc -l)
+ test $n -gt 1
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
