Re: [PATCH] stream_to_pack: xread does not guarantee to read all requested bytes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 20, 2013, 15:00 UTC
- Message-ID
- <xmqqsiy476h4.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <521333AE.1090506@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
Show 24 quoted lines
> 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 <j6t@kdbg.org>
> ---
> 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;