Re: [PATCH] stream_to_pack: xread does not guarantee to read all requested bytes
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:27
Johannes Sixt [off-list ref] writes:
quoted hunk
The deflate loop in bulk-checkin::stream_to_pack expects to get all bytes from a file that it requests to read in a single function call. But it used xread(), which does not give that guarantee. Replace it by read_in_full(). Signed-off-by: Johannes Sixt <redacted> --- The size is limited to sizeof(ibuf) == 16384 bytes, so that there should not be a problem with the unpatched code on any OS in practice. Nevertheless, this change seems reasonable from a code hygiene POV. bulk-checkin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/bulk-checkin.c b/bulk-checkin.c index 6b0b6d4..118c625 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c@@ -114,7 +114,7 @@ static int stream_to_pack(struct bulk_checkin_state *state, if (size && !s.avail_in) { ssize_t rsize = size < sizeof(ibuf) ? size : sizeof(ibuf); - if (xread(fd, ibuf, rsize) != rsize) + if (read_in_full(fd, ibuf, rsize) != rsize)
This is the kind of thing i was wondering and worried about with the other "clipped xread/xwrite" patch. The original of this caller is obviously wrong. Thanks for spotting and fixing. I wonder if there are more like this broken caller or xread and/or xwrite.
die("failed to read %d bytes from '%s'",
(int)rsize, path);
offset += rsize;