git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Be more careful about zlib return values

From
Linus Torvalds <torvalds@linux-foundation.org>
Date
Mar 20, 2007, 18:38 UTC
Message-ID
<Pine.LNX.4.64.0703201124260.6730@woody.linux-foundation.org>

When creating a new object, we use "deflate(stream, Z_FINISH)" in a loop until it no longer returns Z_OK, and then we do "deflateEnd()" to finish up business.

That should all work, but the fact is, it's not how you're _supposed_ to use the zlib return values properly:

 - deflate() should never return Z_OK in the first place, except if we 
   need to increase the output buffer size (which we're not doing, and 
   should never need to do, since we pre-allocated a buffer that is 
   supposed to be able to hold the output in full). So the "while()" loop 
   was incorrect: Z_OK doesn't actually mean "ok, continue", it means "ok, 
   allocate more memory for me and continue"!
 - if we got an error return, we would consider it to be end-of-stream, 
   but it could be some internal zlib error.  In short, we should check 
   for Z_STREAM_END explicitly, since that's the only valid return value 
   anyway for the Z_FINISH case.
 - we never checked deflateEnd() return codes at all.

Now, admittedly, none of these issues should ever happen, unless there is some internal bug in zlib. So this patch should make zero difference, but it seems to be the right thing to do.

We should probablybe anal and check the return value of "deflateInit()" too!

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

Somebody who has worked more with zlib should probably double-check me, but this is what <zlib.h> claims is the right thing to do.

		Linus
---
 sha1_file.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index c445a24..bfcbbea 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1947,7 +1947,7 @@ int hash_sha1_file(void *buf, unsigned long len, const char *type,
 
 int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
 {
-	int size;
+	int size, ret;
 	unsigned char *compressed;
 	z_stream stream;
 	unsigned char sha1[20];
@@ -2007,9 +2007,14 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
 	/* Then the data itself.. */
 	stream.next_in = buf;
 	stream.avail_in = len;
-	while (deflate(&stream, Z_FINISH) == Z_OK)
-		/* nothing */;
-	deflateEnd(&stream);
+	ret = deflate(&stream, Z_FINISH);
+	if (ret != Z_STREAM_END)
+		die("unable to deflate new object %s (%d)", sha1_to_hex(sha1), ret);
+
+	ret = deflateEnd(&stream);
+	if (ret != Z_OK)
+		die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), ret);
+
 	size = stream.total_out;
 
 	if (write_buffer(fd, compressed, size) < 0)
Next: Junio C Hamano
Message 1 of 3 in “Be more careful about zlib return values”
  1. Linus TorvaldsMar 20, 2007
  2. Junio C HamanoMar 21, 2007
  3. Linus TorvaldsMar 21, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.