Wrap inflate and other zlib routines for better error reporting
R. Tyler Ballance reported a mysterious transient repository corruption;
after much digging, it turns out that we were not catching and reporting
memory allocation errors from some calls we make to zlib.
This one _just_ wraps things; it doesn't do the "retry on low memory
error" part, at least not yet. It is an independent issue from the
reporting. Some of the errors are expected and passed back to the caller,
but we die when zlib reports it failed to allocate memory for now.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/builtin-apply.c b/builtin-apply.c
index 50b623e..eface97 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1258,8 +1258,9 @@
stream.avail_in = size;
stream.next_out = out = xmalloc(inflated_size);
stream.avail_out = inflated_size;
- inflateInit(&stream);
- st = inflate(&stream, Z_FINISH);
+ git_inflate_init(&stream);
+ st = git_inflate(&stream, Z_FINISH);
+ git_inflate_end(&stream);
if ((st != Z_STREAM_END) || stream.total_out != inflated_size) {
free(out);
return NULL;
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index fb5e14d..85af80f 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -195,16 +195,16 @@
int st;
memset(&stream, 0, sizeof(stream));
- inflateInit(&stream);
+ git_inflate_init(&stream);
do {
in = use_pack(p, w_curs, offset, &stream.avail_in);
stream.next_in = in;
stream.next_out = fakebuf;
stream.avail_out = sizeof(fakebuf);
- st = inflate(&stream, Z_FINISH);
+ st = git_inflate(&stream, Z_FINISH);
offset += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR);
- inflateEnd(&stream);
+ git_inflate_end(&stream);
return (st == Z_STREAM_END &&
stream.total_out == expect &&
stream.total_in == len) ? 0 : -1;
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 40b20f2..41d00d3 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -99,10 +99,10 @@
stream.avail_out = size;
stream.next_in = fill(1);
stream.avail_in = len;
- inflateInit(&stream);
+ git_inflate_init(&stream);
for (;;) {
- int ret = inflate(&stream, 0);
+ int ret = git_inflate(&stream, 0);
use(len - stream.avail_in);
if (stream.total_out == size && ret == Z_STREAM_END)
break;
@@ -118,7 +118,7 @@
stream.next_in = fill(1);
stream.avail_in = len;
}
- inflateEnd(&stream);
+ git_inflate_end(&stream);
return buf;
}
diff --git a/cache.h b/cache.h
index 42f2f27..17ce444 100644
--- a/cache.h
+++ b/cache.h
@@ -12,6 +12,10 @@
#define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
#endif
+void git_inflate_init(z_streamp strm);
+void git_inflate_end(z_streamp strm);
+int git_inflate(z_streamp strm, int flush);
+
#if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)
#define DTYPE(de) ((de)->d_type)
#else
diff --git a/http-push.c b/http-push.c
index 6805288..6d977ea 100644
--- a/http-push.c
+++ b/http-push.c
@@ -208,7 +208,7 @@
do {
request->stream.next_out = expn;
request->stream.avail_out = sizeof(expn);
- request->zret = inflate(&request->stream, Z_SYNC_FLUSH);
+ request->zret = git_inflate(&request->stream, Z_SYNC_FLUSH);
SHA1_Update(&request->c, expn,
sizeof(expn) - request->stream.avail_out);
} while (request->stream.avail_in && request->zret == Z_OK);
@@ -268,7 +268,7 @@
memset(&request->stream, 0, sizeof(request->stream));
- inflateInit(&request->stream);
+ git_inflate_init(&request->stream);
SHA1_Init(&request->c);
@@ -309,7 +309,7 @@
file; also rewind to the beginning of the local file. */
if (prev_read == -1) {
memset(&request->stream, 0, sizeof(request->stream));
- inflateInit(&request->stream);
+ git_inflate_init(&request->stream);
SHA1_Init(&request->c);
if (prev_posn>0) {
prev_posn = 0;
@@ -741,7 +741,7 @@
if (request->http_code == 416)
fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n");
- inflateEnd(&request->stream);
+ git_inflate_end(&request->stream);
SHA1_Final(request->real_sha1, &request->c);
if (request->zret != Z_STREAM_END) {
unlink(request->tmpfile);
diff --git a/http-walker.c b/http-walker.c
index 9dc6b27..747d3ad 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -82,7 +82,7 @@
do {
obj_req->stream.next_out = expn;
obj_req->stream.avail_out = sizeof(expn);
- obj_req->zret = inflate(&obj_req->stream, Z_SYNC_FLUSH);
+ obj_req->zret = git_inflate(&obj_req->stream, Z_SYNC_FLUSH);
SHA1_Update(&obj_req->c, expn,
sizeof(expn) - obj_req->stream.avail_out);
} while (obj_req->stream.avail_in && obj_req->zret == Z_OK);
@@ -142,7 +142,7 @@
memset(&obj_req->stream, 0, sizeof(obj_req->stream));
- inflateInit(&obj_req->stream);
+ git_inflate_init(&obj_req->stream);
SHA1_Init(&obj_req->c);
@@ -183,7 +183,7 @@
file; also rewind to the beginning of the local file. */
if (prev_read == -1) {
memset(&obj_req->stream, 0, sizeof(obj_req->stream));
- inflateInit(&obj_req->stream);
+ git_inflate_init(&obj_req->stream);
SHA1_Init(&obj_req->c);
if (prev_posn>0) {
prev_posn = 0;
@@ -243,7 +243,7 @@
return;
}
- inflateEnd(&obj_req->stream);
+ git_inflate_end(&obj_req->stream);
SHA1_Final(obj_req->real_sha1, &obj_req->c);
if (obj_req->zret != Z_STREAM_END) {
unlink(obj_req->tmpfile);
diff --git a/index-pack.c b/index-pack.c
index c99a1a1..c2d17bf 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -271,10 +271,10 @@
stream.avail_out = size;
stream.next_in = fill(1);
stream.avail_in = input_len;
- inflateInit(&stream);
+ git_inflate_init(&stream);
for (;;) {
- int ret = inflate(&stream, 0);
+ int ret = git_inflate(&stream, 0);
use(input_len - stream.avail_in);
if (stream.total_out == size && ret == Z_STREAM_END)
break;
@@ -283,7 +283,7 @@
stream.next_in = fill(1);
stream.avail_in = input_len;
}
- inflateEnd(&stream);
+ git_inflate_end(&stream);
return buf;
}
@@ -378,9 +378,9 @@
stream.avail_out = obj->size;
stream.next_in = src;
stream.avail_in = len;
- inflateInit(&stream);
- while ((st = inflate(&stream, Z_FINISH)) == Z_OK);
- inflateEnd(&stream);
+ git_inflate_init(&stream);
+ while ((st = git_inflate(&stream, Z_FINISH)) == Z_OK);
+ git_inflate_end(&stream);
if (st != Z_STREAM_END || stream.total_out != obj->size)
die("serious inflate inconsistency");
free(src);
diff --git a/sha1_file.c b/sha1_file.c
index 88035a0..71c2282 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1182,8 +1182,8 @@
stream->avail_out = bufsiz;
if (legacy_loose_object(map)) {
- inflateInit(stream);
- return inflate(stream, 0);
+ git_inflate_init(stream);
+ return git_inflate(stream, 0);
}
@@ -1203,7 +1203,7 @@
/* Set up the stream for the rest.. */
stream->next_in = map;
stream->avail_in = mapsize;
- inflateInit(stream);
+ git_inflate_init(stream);
/* And generate the fake traditional header */
stream->total_out = 1 + snprintf(buffer, bufsiz, "%s %lu",
@@ -1240,11 +1240,11 @@
stream->next_out = buf + bytes;
stream->avail_out = size - bytes;
while (status == Z_OK)
- status = inflate(stream, Z_FINISH);
+ status = git_inflate(stream, Z_FINISH);
}
buf[size] = 0;
if (status == Z_STREAM_END && !stream->avail_in) {
- inflateEnd(stream);
+ git_inflate_end(stream);
return buf;
}
@@ -1334,15 +1334,15 @@
stream.next_out = delta_head;
stream.avail_out = sizeof(delta_head);
- inflateInit(&stream);
+ git_inflate_init(&stream);
do {
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
- st = inflate(&stream, Z_FINISH);
+ st = git_inflate(&stream, Z_FINISH);
curpos += stream.next_in - in;
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
stream.total_out < sizeof(delta_head));
- inflateEnd(&stream);
+ git_inflate_end(&stream);
if ((st != Z_STREAM_END) && stream.total_out != sizeof(delta_head))
die("delta data unpack-initial failed");
@@ -1550,14 +1550,14 @@
stream.next_out = buffer;
stream.avail_out = size;
- inflateInit(&stream);
+ git_inflate_init(&stream);
do {
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
- st = inflate(&stream, Z_FINISH);
+ st = git_inflate(&stream, Z_FINISH);
curpos += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR);
- inflateEnd(&stream);
+ git_inflate_end(&stream);
if ((st != Z_STREAM_END) || stream.total_out != size) {
free(buffer);
return NULL;
@@ -1965,7 +1965,7 @@
status = error("unable to parse %s header", sha1_to_hex(sha1));
else if (sizep)
*sizep = size;
- inflateEnd(&stream);
+ git_inflate_end(&stream);
munmap(map, mapsize);
return status;
}
diff --git a/wrapper.c b/wrapper.c
index 93562f0..c85ca52 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -196,3 +196,63 @@
die("Unable to create temporary file: %s", strerror(errno));
return fd;
}
+
+/*
+ * zlib wrappers to make sure we don't silently miss errors
+ * at init time.
+ */
+void git_inflate_init(z_streamp strm)
+{
+ const char *err;
+
+ switch (inflateInit(strm)) {
+ case Z_OK:
+ return;
+
+ case Z_MEM_ERROR:
+ err = "out of memory";
+ break;
+ case Z_VERSION_ERROR:
+ err = "wrong version";
+ break;
+ default:
+ err = "error";
+ }
+ die("inflateInit: %s (%s)", err, strm->msg ? strm->msg : "no message");
+}
+
+void git_inflate_end(z_streamp strm)
+{
+ if (inflateEnd(strm) != Z_OK)
+ error("inflateEnd: %s", strm->msg ? strm->msg : "failed");
+}
+
+int git_inflate(z_streamp strm, int flush)
+{
+ int ret = inflate(strm, flush);
+ const char *err;
+
+ switch (ret) {
+ /* Out of memory is fatal. */
+ case Z_MEM_ERROR:
+ die("inflate: out of memory");
+
+ /* Data corruption errors: we may want to recover from them (fsck) */
+ case Z_NEED_DICT:
+ err = "needs dictionary"; break;
+ case Z_DATA_ERROR:
+ err = "data stream error"; break;
+ case Z_STREAM_ERROR:
+ err = "stream consistency error"; break;
+ default:
+ err = "unknown error"; break;
+
+ /* Z_BUF_ERROR: normal, needs more space in the output buffer */
+ case Z_BUF_ERROR:
+ case Z_OK:
+ case Z_STREAM_END:
+ return ret;
+ }
+ error("inflate: %s (%s)", err, strm->msg ? strm->msg : "no message");
+ return ret;
+}
