Re: [PATCHv3 02/13] xread: poll on non blocking fds

["Torsten Bögershausen" <tboegi@web.de>]

On 09/22/2015 01:55 AM, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> So if we get an EAGAIN or EWOULDBLOCK error the fd must be nonblocking.
>> As the intend of xread is to read as much as possible either until the
>> fd is EOF or an actual error occurs, we can ease the feeder of the fd
>> by not spinning the whole time, but rather wait for it politely by not
>> busy waiting.
> As you said in the cover letter, this does look questionable.  It is
> sweeping the problem under the rug (the hard-coded 100ms is a good
> clue to tell that).  If the caller does want us to read thru to the
> end, then we would need to make it easier for such a caller to stop
> marking the file descriptor to be non-blocking, but this does not do
> anything to help that.  An alternative might be to automatically
> turn nonblocking off temporarily once we get EAGAIN (and turn it on
> again before leaving); that would be an approach to make it
> unnecessary to fix the caller (which has its own set of problems,
> though).
Wouldn'  that function be somewhat mis-named and/or mis-behaved?
read_in_full_with_hard_coded_timeout_and_fix_O_NONBLOCK()
could make sure that the user of this function knows what's going on
under the hood.

More seriously, if someone calls xread() with a non-blocking socket,
the caller wants to return and does want to his own timeout handling.

If we want to have a timeouted read(), we can call it

xread_timout(int fd, voxread(int fd, void *buf, size_t len, int timeout)

(Or something similar) to make clear that there is an underlying
timeout handling.
Another option could be to name the function

read_in_full_timeout().

But in any case I suggest to  xread() as it is, and not to change the 
functionality
behind the back of the users.