Thread (1 message) 1 message, 1 author, 2017-10-10

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help