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