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 50b623e54c003fb5bbe183568d13f496c3c41c25..eface9793ad9d4dd736161c54b9ee4a8af461218 100644 (file)
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1258,8 +1258,9 @@ static char *inflate_it(const void *data, unsigned long size,
        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 fb5e14d56e5b9fec0a054eb9708be41a59ac0f13..85af80f229ef2f1be78ba1341edcbfeb5d0fdbff 100644 (file)
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -195,16 +195,16 @@ static int check_pack_inflate(struct packed_git *p,
        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 40b20f26e86acca2ee37b34519e84a1ce79689c3..41d00d36cd10cbbb775848c1a7e58330f09891b0 100644 (file)
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -99,10 +99,10 @@ static void *get_data(unsigned long size)
        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 @@ static void *get_data(unsigned long size)
                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 42f2f2754b93ff9d93883529778190d35a8ea43a..17ce4441b6a00e7f86774928954385fefa4d60dc 100644 (file)
--- 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 68052888570af7d09535db8831b8cf3ef2881589..6d977ea6e17ce3a03e47fbfdbc83f8d33f6dd94b 100644 (file)
--- a/http-push.c
+++ b/http-push.c
@@ -208,7 +208,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb,
        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 @@ static void start_fetch_loose(struct transfer_request *request)
 
        memset(&request->stream, 0, sizeof(request->stream));
 
-       inflateInit(&request->stream);
+       git_inflate_init(&request->stream);
 
        SHA1_Init(&request->c);
 
@@ -309,7 +309,7 @@ static void start_fetch_loose(struct transfer_request *request)
           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 @@ static void finish_request(struct transfer_request *request)
                        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 9dc6b27b457a2979a95018679a0b885e6fb62d9a..747d3adef884ec4d5c0370fc5c086e1904a7d442 100644 (file)
--- a/http-walker.c
+++ b/http-walker.c
@@ -82,7 +82,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb,
        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 @@ static void start_object_request(struct walker *walker,
 
        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 @@ static void start_object_request(struct walker *walker,
           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 @@ static void finish_object_request(struct object_request *obj_req)
                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 c99a1a152c7ddbb63073461b86179f2be5bf9640..c2d17bf61326190733620347812901fd791b0e63 100644 (file)
--- a/index-pack.c
+++ b/index-pack.c
@@ -271,10 +271,10 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size)
        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 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size)
                stream.next_in = fill(1);
                stream.avail_in = input_len;
        }
-       inflateEnd(&stream);
+       git_inflate_end(&stream);
        return buf;
 }
 
@@ -378,9 +378,9 @@ static void *get_data_from_pack(struct object_entry *obj)
        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 88035a0cd19d040c8b626684c3ff1f7fdb35e837..71c2282c9c56a00be75e3bc395e6e695dae229f2 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1182,8 +1182,8 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
        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 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
        /* 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 @@ static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size
                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 @@ unsigned long get_size_from_delta(struct packed_git *p,
        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 @@ static void *unpack_compressed_entry(struct packed_git *p,
        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 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size
                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 93562f03eef21b26945d2d9bbdc96818f4de6567..c85ca52ec63a679a2da7bd8980ad4e2df4e38794 100644 (file)
--- a/wrapper.c
+++ b/wrapper.c
@@ -196,3 +196,63 @@ int xmkstemp(char *template)
                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;
+}