Re: [PATCH] stream_to_pack: xread does not guarantee to read all requested bytes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Sixt <j6t@xxxxxxxx> writes:

> 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@xxxxxxxx>
> ---
>  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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]