Re: [PATCH] sha1_loose_object_info: handle errors from unpack_sha1_rest
From: Junio C Hamano <hidden>
Date: 2017-10-10 01:34:15
Jeff King [off-list ref] writes:
quoted hunk
Actually, there are only two callers left these days. One of them leaks, and the other immediately closes the zstream. So something like:diff --git a/sha1_file.c b/sha1_file.c index 09ad64ce55..cea003d182 100644 --- a/sha1_file.c +++ b/sha1_file.c@@ -978,10 +978,10 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s while (status == Z_OK) status = git_inflate(stream, Z_FINISH); } - if (status == Z_STREAM_END && !stream->avail_in) { - git_inflate_end(stream); + git_inflate_end(stream); + + if (status == Z_STREAM_END && !stream->avail_in) return buf; - } if (status < 0) error("corrupt loose object '%s'", sha1_to_hex(sha1));@@ -2107,7 +2107,6 @@ int read_loose_object(const char *path, *contents = unpack_sha1_rest(&stream, hdr, *size, expected_sha1); if (!*contents) { error("unable to unpack contents of %s", path); - git_inflate_end(&stream); goto out; } if (check_sha1_signature(expected_sha1, *contents,seems reasonable. Doing it that (with my other patch on top) splits the leak-fix and the not-yet-a-bug-but-confusing-error-return problems into two separate patches. I dunno. There aren't that many callers of unpack_sha1_rest(), so it may not matter that much, but while we're here...
Yeah, I agree that it is a reasonable thing to do.