Skip to content

Commit 39c6854

Browse files
torvaldsgitster
authored andcommitted
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 <[email protected]>
1 parent 141201d commit 39c6854

File tree

9 files changed

+99
-34
lines changed

9 files changed

+99
-34
lines changed

builtin-apply.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,8 +1258,9 @@ static char *inflate_it(const void *data, unsigned long size,
12581258
stream.avail_in = size;
12591259
stream.next_out = out = xmalloc(inflated_size);
12601260
stream.avail_out = inflated_size;
1261-
inflateInit(&stream);
1262-
st = inflate(&stream, Z_FINISH);
1261+
git_inflate_init(&stream);
1262+
st = git_inflate(&stream, Z_FINISH);
1263+
git_inflate_end(&stream);
12631264
if ((st != Z_STREAM_END) || stream.total_out != inflated_size) {
12641265
free(out);
12651266
return NULL;

builtin-pack-objects.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,16 +195,16 @@ static int check_pack_inflate(struct packed_git *p,
195195
int st;
196196

197197
memset(&stream, 0, sizeof(stream));
198-
inflateInit(&stream);
198+
git_inflate_init(&stream);
199199
do {
200200
in = use_pack(p, w_curs, offset, &stream.avail_in);
201201
stream.next_in = in;
202202
stream.next_out = fakebuf;
203203
stream.avail_out = sizeof(fakebuf);
204-
st = inflate(&stream, Z_FINISH);
204+
st = git_inflate(&stream, Z_FINISH);
205205
offset += stream.next_in - in;
206206
} while (st == Z_OK || st == Z_BUF_ERROR);
207-
inflateEnd(&stream);
207+
git_inflate_end(&stream);
208208
return (st == Z_STREAM_END &&
209209
stream.total_out == expect &&
210210
stream.total_in == len) ? 0 : -1;

builtin-unpack-objects.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,10 @@ static void *get_data(unsigned long size)
9999
stream.avail_out = size;
100100
stream.next_in = fill(1);
101101
stream.avail_in = len;
102-
inflateInit(&stream);
102+
git_inflate_init(&stream);
103103

104104
for (;;) {
105-
int ret = inflate(&stream, 0);
105+
int ret = git_inflate(&stream, 0);
106106
use(len - stream.avail_in);
107107
if (stream.total_out == size && ret == Z_STREAM_END)
108108
break;
@@ -118,7 +118,7 @@ static void *get_data(unsigned long size)
118118
stream.next_in = fill(1);
119119
stream.avail_in = len;
120120
}
121-
inflateEnd(&stream);
121+
git_inflate_end(&stream);
122122
return buf;
123123
}
124124

cache.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
#define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
1313
#endif
1414

15+
void git_inflate_init(z_streamp strm);
16+
void git_inflate_end(z_streamp strm);
17+
int git_inflate(z_streamp strm, int flush);
18+
1519
#if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)
1620
#define DTYPE(de) ((de)->d_type)
1721
#else

http-push.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb,
208208
do {
209209
request->stream.next_out = expn;
210210
request->stream.avail_out = sizeof(expn);
211-
request->zret = inflate(&request->stream, Z_SYNC_FLUSH);
211+
request->zret = git_inflate(&request->stream, Z_SYNC_FLUSH);
212212
SHA1_Update(&request->c, expn,
213213
sizeof(expn) - request->stream.avail_out);
214214
} while (request->stream.avail_in && request->zret == Z_OK);
@@ -268,7 +268,7 @@ static void start_fetch_loose(struct transfer_request *request)
268268

269269
memset(&request->stream, 0, sizeof(request->stream));
270270

271-
inflateInit(&request->stream);
271+
git_inflate_init(&request->stream);
272272

273273
SHA1_Init(&request->c);
274274

@@ -309,7 +309,7 @@ static void start_fetch_loose(struct transfer_request *request)
309309
file; also rewind to the beginning of the local file. */
310310
if (prev_read == -1) {
311311
memset(&request->stream, 0, sizeof(request->stream));
312-
inflateInit(&request->stream);
312+
git_inflate_init(&request->stream);
313313
SHA1_Init(&request->c);
314314
if (prev_posn>0) {
315315
prev_posn = 0;
@@ -741,7 +741,7 @@ static void finish_request(struct transfer_request *request)
741741
if (request->http_code == 416)
742742
fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n");
743743

744-
inflateEnd(&request->stream);
744+
git_inflate_end(&request->stream);
745745
SHA1_Final(request->real_sha1, &request->c);
746746
if (request->zret != Z_STREAM_END) {
747747
unlink(request->tmpfile);

http-walker.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb,
8282
do {
8383
obj_req->stream.next_out = expn;
8484
obj_req->stream.avail_out = sizeof(expn);
85-
obj_req->zret = inflate(&obj_req->stream, Z_SYNC_FLUSH);
85+
obj_req->zret = git_inflate(&obj_req->stream, Z_SYNC_FLUSH);
8686
SHA1_Update(&obj_req->c, expn,
8787
sizeof(expn) - obj_req->stream.avail_out);
8888
} while (obj_req->stream.avail_in && obj_req->zret == Z_OK);
@@ -142,7 +142,7 @@ static void start_object_request(struct walker *walker,
142142

143143
memset(&obj_req->stream, 0, sizeof(obj_req->stream));
144144

145-
inflateInit(&obj_req->stream);
145+
git_inflate_init(&obj_req->stream);
146146

147147
SHA1_Init(&obj_req->c);
148148

@@ -183,7 +183,7 @@ static void start_object_request(struct walker *walker,
183183
file; also rewind to the beginning of the local file. */
184184
if (prev_read == -1) {
185185
memset(&obj_req->stream, 0, sizeof(obj_req->stream));
186-
inflateInit(&obj_req->stream);
186+
git_inflate_init(&obj_req->stream);
187187
SHA1_Init(&obj_req->c);
188188
if (prev_posn>0) {
189189
prev_posn = 0;
@@ -243,7 +243,7 @@ static void finish_object_request(struct object_request *obj_req)
243243
return;
244244
}
245245

246-
inflateEnd(&obj_req->stream);
246+
git_inflate_end(&obj_req->stream);
247247
SHA1_Final(obj_req->real_sha1, &obj_req->c);
248248
if (obj_req->zret != Z_STREAM_END) {
249249
unlink(obj_req->tmpfile);

index-pack.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,10 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size)
271271
stream.avail_out = size;
272272
stream.next_in = fill(1);
273273
stream.avail_in = input_len;
274-
inflateInit(&stream);
274+
git_inflate_init(&stream);
275275

276276
for (;;) {
277-
int ret = inflate(&stream, 0);
277+
int ret = git_inflate(&stream, 0);
278278
use(input_len - stream.avail_in);
279279
if (stream.total_out == size && ret == Z_STREAM_END)
280280
break;
@@ -283,7 +283,7 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size)
283283
stream.next_in = fill(1);
284284
stream.avail_in = input_len;
285285
}
286-
inflateEnd(&stream);
286+
git_inflate_end(&stream);
287287
return buf;
288288
}
289289

@@ -378,9 +378,9 @@ static void *get_data_from_pack(struct object_entry *obj)
378378
stream.avail_out = obj->size;
379379
stream.next_in = src;
380380
stream.avail_in = len;
381-
inflateInit(&stream);
382-
while ((st = inflate(&stream, Z_FINISH)) == Z_OK);
383-
inflateEnd(&stream);
381+
git_inflate_init(&stream);
382+
while ((st = git_inflate(&stream, Z_FINISH)) == Z_OK);
383+
git_inflate_end(&stream);
384384
if (st != Z_STREAM_END || stream.total_out != obj->size)
385385
die("serious inflate inconsistency");
386386
free(src);

sha1_file.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,8 +1182,8 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
11821182
stream->avail_out = bufsiz;
11831183

11841184
if (legacy_loose_object(map)) {
1185-
inflateInit(stream);
1186-
return inflate(stream, 0);
1185+
git_inflate_init(stream);
1186+
return git_inflate(stream, 0);
11871187
}
11881188

11891189

@@ -1203,7 +1203,7 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
12031203
/* Set up the stream for the rest.. */
12041204
stream->next_in = map;
12051205
stream->avail_in = mapsize;
1206-
inflateInit(stream);
1206+
git_inflate_init(stream);
12071207

12081208
/* And generate the fake traditional header */
12091209
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
12401240
stream->next_out = buf + bytes;
12411241
stream->avail_out = size - bytes;
12421242
while (status == Z_OK)
1243-
status = inflate(stream, Z_FINISH);
1243+
status = git_inflate(stream, Z_FINISH);
12441244
}
12451245
buf[size] = 0;
12461246
if (status == Z_STREAM_END && !stream->avail_in) {
1247-
inflateEnd(stream);
1247+
git_inflate_end(stream);
12481248
return buf;
12491249
}
12501250

@@ -1334,15 +1334,15 @@ unsigned long get_size_from_delta(struct packed_git *p,
13341334
stream.next_out = delta_head;
13351335
stream.avail_out = sizeof(delta_head);
13361336

1337-
inflateInit(&stream);
1337+
git_inflate_init(&stream);
13381338
do {
13391339
in = use_pack(p, w_curs, curpos, &stream.avail_in);
13401340
stream.next_in = in;
1341-
st = inflate(&stream, Z_FINISH);
1341+
st = git_inflate(&stream, Z_FINISH);
13421342
curpos += stream.next_in - in;
13431343
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
13441344
stream.total_out < sizeof(delta_head));
1345-
inflateEnd(&stream);
1345+
git_inflate_end(&stream);
13461346
if ((st != Z_STREAM_END) && stream.total_out != sizeof(delta_head))
13471347
die("delta data unpack-initial failed");
13481348

@@ -1550,14 +1550,14 @@ static void *unpack_compressed_entry(struct packed_git *p,
15501550
stream.next_out = buffer;
15511551
stream.avail_out = size;
15521552

1553-
inflateInit(&stream);
1553+
git_inflate_init(&stream);
15541554
do {
15551555
in = use_pack(p, w_curs, curpos, &stream.avail_in);
15561556
stream.next_in = in;
1557-
st = inflate(&stream, Z_FINISH);
1557+
st = git_inflate(&stream, Z_FINISH);
15581558
curpos += stream.next_in - in;
15591559
} while (st == Z_OK || st == Z_BUF_ERROR);
1560-
inflateEnd(&stream);
1560+
git_inflate_end(&stream);
15611561
if ((st != Z_STREAM_END) || stream.total_out != size) {
15621562
free(buffer);
15631563
return NULL;
@@ -1965,7 +1965,7 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size
19651965
status = error("unable to parse %s header", sha1_to_hex(sha1));
19661966
else if (sizep)
19671967
*sizep = size;
1968-
inflateEnd(&stream);
1968+
git_inflate_end(&stream);
19691969
munmap(map, mapsize);
19701970
return status;
19711971
}

wrapper.c

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,3 +196,63 @@ int xmkstemp(char *template)
196196
die("Unable to create temporary file: %s", strerror(errno));
197197
return fd;
198198
}
199+
200+
/*
201+
* zlib wrappers to make sure we don't silently miss errors
202+
* at init time.
203+
*/
204+
void git_inflate_init(z_streamp strm)
205+
{
206+
const char *err;
207+
208+
switch (inflateInit(strm)) {
209+
case Z_OK:
210+
return;
211+
212+
case Z_MEM_ERROR:
213+
err = "out of memory";
214+
break;
215+
case Z_VERSION_ERROR:
216+
err = "wrong version";
217+
break;
218+
default:
219+
err = "error";
220+
}
221+
die("inflateInit: %s (%s)", err, strm->msg ? strm->msg : "no message");
222+
}
223+
224+
void git_inflate_end(z_streamp strm)
225+
{
226+
if (inflateEnd(strm) != Z_OK)
227+
error("inflateEnd: %s", strm->msg ? strm->msg : "failed");
228+
}
229+
230+
int git_inflate(z_streamp strm, int flush)
231+
{
232+
int ret = inflate(strm, flush);
233+
const char *err;
234+
235+
switch (ret) {
236+
/* Out of memory is fatal. */
237+
case Z_MEM_ERROR:
238+
die("inflate: out of memory");
239+
240+
/* Data corruption errors: we may want to recover from them (fsck) */
241+
case Z_NEED_DICT:
242+
err = "needs dictionary"; break;
243+
case Z_DATA_ERROR:
244+
err = "data stream error"; break;
245+
case Z_STREAM_ERROR:
246+
err = "stream consistency error"; break;
247+
default:
248+
err = "unknown error"; break;
249+
250+
/* Z_BUF_ERROR: normal, needs more space in the output buffer */
251+
case Z_BUF_ERROR:
252+
case Z_OK:
253+
case Z_STREAM_END:
254+
return ret;
255+
}
256+
error("inflate: %s (%s)", err, strm->msg ? strm->msg : "no message");
257+
return ret;
258+
}

0 commit comments

Comments
 (0)