Re: [PATCH v4 1/4] transport-helper: use xread instead of read

[Jeff King <peff@peff.net>]

On Wed, Jan 02, 2019 at 12:55:51PM -0800, Junio C Hamano wrote:

> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> > ---
> >  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@sigill.intra.peff.net/

(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.

-Peff