Skip to content

Commit

Permalink
Wrap inflate and other zlib routines for better error reporting
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Linus Torvalds authored and Junio C Hamano committed Jan 11, 2009
1 parent 141201d commit 39c6854
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 34 deletions.
5 changes: 3 additions & 2 deletions builtin-apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions builtin-pack-objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions builtin-unpack-objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
4 changes: 4 additions & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions http-push.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions http-walker.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions index-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
24 changes: 12 additions & 12 deletions sha1_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand All @@ -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",
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
60 changes: 60 additions & 0 deletions wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 39c6854

Please sign in to comment.