Re: [PATCH] read_in_full: always report errors

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Subject: [PATCH] read_in_full: always report errors
>
> The read_in_full function repeatedly calls read() to fill a
> buffer. If the first read() returns an error, we notify the
> caller by returning the error. However, if we read some data
> and then get an error on a subsequent read, we simply return
> the amount of data that we did read, and the caller is
> unaware of the error.

Is the caller unaware?  While it won't hurt the callers who do:

   if (expect != read_in_full(fd, buf, expect))
      die(...)

I think this change hurts the one that you mentioned in your analysis.

The caller in index_stream() reads what it could, writes what it read, and
comes back and makes another call to read_in_full(), at which point either
it gets an error and the whole thing would error out (i.e. no difference
from before), or if it was an transient error that interrupted the
previous read_in_full(), it can keep reading (with this patch it will not
have a chance to do so).

> This makes the tradeoff that seeing the partial data is more
> important than the fact that an error occurred. In practice,
> this is generally not the case; we care more if an error
> occurred, and should throw away any partial data.

Not really. I think we care about both, and I think that is what the
current code tries to do.