Jeff King <peff@xxxxxxxx> writes:
> On Wed, Jan 02, 2019 at 12:55:51PM -0800, Junio C Hamano wrote:
>
>> > Signed-off-by: Randall S. Becker <rsbecker@xxxxxxxxxxxxx>
>> > ---
>> > transport-helper.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/transport-helper.c b/transport-helper.c
>> > index bf225c698f..a290695a12 100644
>> > --- a/transport-helper.c
>> > +++ b/transport-helper.c
>> > @@ -1225,7 +1225,7 @@ static int udt_do_read(struct unidirectional_transfer *t)
>> > return 0; /* No space for more. */
>> >
>> > transfer_debug("%s is readable", t->src_name);
>> > - bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
>> > + bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
>> > - if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
>> > - errno != EINTR) {
>> > + if (bytes < 0 && errno != EINTR) {
>> > error_errno(_("read(%s) failed"), t->src_name);
>>
>> Can't we also lose EINTR check, though? When read() returns
>> negative, we check errno and if it is EINTR, continue the loop.
>
> Yes.
>
> I also wondered if this caller might actually be relying on the current
> non-looping behavior, but it looks like I already traced through and
> determined this was OK:
>
> https://public-inbox.org/git/20180111063110.GB31213@xxxxxxxxxxxxxxxxxxxxx/
>
> (the cleanup is correct either way, but that is what makes the
> conversion to xread() OK).
>
> We may want to just take the xread() conversion and then do the patch
> that I linked above on top, since it also cleans up the xwrite() spot,
> too.
OK. That does sound cleaner.
