Re: [PATCHv2 2/7] xread: poll on non blocking fds

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

On 16.12.15 01:04, Stefan Beller wrote:
> The man page of read(2) says:
> 
>   EAGAIN The file descriptor fd refers to a file other than a socket
> 	 and has been marked nonblocking (O_NONBLOCK), and the read
> 	 would block.
> 
>   EAGAIN or EWOULDBLOCK
> 	 The file descriptor fd refers to a socket and has been marked
> 	 nonblocking (O_NONBLOCK), and the read would block.  POSIX.1-2001
> 	 allows either error to be returned for this case, and does not
> 	 require these constants to have the same value, so a portable
> 	 application should check for both possibilities.
> 
> If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.
> As the intent 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.
> 
> We should not care if the call to poll failed, as we're in an infinite
> loop and can only get out with the correct read().
I'm not sure if this is valid under all circumstances:
This is what "man poll" says under Linux: 
[]
 ENOMEM There was no space to allocate file descriptor tables.
[]
And this is from Mac OS, ("BSD System Calls Manual")
ERRORS
     Poll() will fail if:

     [EAGAIN]           Allocation of internal data structures fails.  A sub-
                        sequent request may succeed.
And this is opengroup:
http://pubs.opengroup.org/onlinepubs/9699919799//functions/poll.html:
[EAGAIN]
    The allocation of internal data structures failed but a subsequent request may succeed.

read() may return EAGAIN, but poll() may fail to allocate memory, and fail.
Is it always guaranteed that the loop is terminated?



> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Acked-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  wrapper.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/wrapper.c b/wrapper.c
> index 6fcaa4d..1770efa 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -236,8 +236,24 @@ ssize_t xread(int fd, void *buf, size_t len)
>  	    len = MAX_IO_SIZE;
>  	while (1) {
>  		nr = read(fd, buf, len);
> -		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
> -			continue;
> +		if (nr < 0) {
> +			if (errno == EINTR)
> +				continue;
> +			if (errno == EAGAIN || errno == EWOULDBLOCK) {
> +				struct pollfd pfd;
> +				pfd.events = POLLIN;
> +				pfd.fd = fd;
> +				/*
> +				 * it is OK if this poll() failed; we
> +				 * want to leave this infinite loop
> +				 * only when read() returns with
> +				 * success, or an expected failure,
> +				 * which would be checked by the next
> +				 * call to read(2).
> +				 */
> +				poll(&pfd, 1, -1);
> +			}
> +		}
>  		return nr;
>  	}
>  }
>