From: Linus Torvalds <torvalds@linux-foundation.org>
To: Junio C Hamano <junkio@cox.net>, Git Mailing List <git@vger.kernel.org>
Subject: Be more careful about zlib return values
Date: Tue, 20 Mar 2007 11:38:34 -0700 (PDT) [thread overview]
Message-ID: <Pine.LNX.4.64.0703201124260.6730@woody.linux-foundation.org> (raw)
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 reply other threads:[~2007-03-20 18:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-03-20 18:38 Linus Torvalds [this message]
2007-03-21 8:11 ` Be more careful about zlib return values Junio C Hamano
2007-03-21 15:29 ` Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.0703201124260.6730@woody.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=git@vger.kernel.org \
--cc=junkio@cox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).