Re: [PATCH 01/10] strbuf: Add strbuf_read_noblock

[Jeff King <peff@peff.net>]

On Thu, Sep 17, 2015 at 09:13:40AM -0700, Junio C Hamano wrote:

> And your new caller that does O_NONBLOCK wants to do more than
> looping upon EWOULDBLOCK.  It certainly would not want us to loop
> here.
> 
> So I wonder if you can just O_NONBLOCK the fd and use the usual
> strbuf_read(), i.e. without any change in this patch, and update
> xread() to _unconditionally_ return when read(2) says EAGAIN or
> EWOULDBLOCK.
> 
> What would that break?

Certainly anybody who does not realize their descriptor is O_NONBLOCK
and is using the spinning for correctness. I tend to think that such
sites are wrong, though, and would benefit from us realizing they are
spinning.

But I think you can't quite get away with leaving strbuf_read untouched
in this case. On error, it wants to restore the original value of the
strbuf before the strbuf_read call. Which means that we throw away
anything read into the strbuf before we get EAGAIN, and the caller never
gets to see it.

So I think we would probably want to treat EAGAIN specially: return -1
to signal to the caller but _don't_ truncate the strbuf.

Arguably we should actually return the number of bytes we _did_ read,
but then caller cannot easily tell the difference between EOF and
EAGAIN.

-Peff