Re: [PATCHv2 2/7] xread: poll on non blocking fds
- From
Torsten Bögershausen <tboegi@web.de>
- Date
- Dec 17, 2015, 20:12 UTC
- Message-ID
- <56731715.9000509@web.de>
- In-Reply-To
- <1450224252-16833-3-git-send-email-sbeller@google.com>
On 16.12.15 01:04, Stefan Beller wrote:
Show 21 quoted lines
> 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?
Show 40 quoted lines
>
> 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;
> }
> }
>