Re: [PATCHv2 2/7] xread: poll on non blocking fds
- From
Stefan Beller <sbeller@google.com>
- Date
- Dec 17, 2015, 20:22 UTC
- Message-ID
- <CAGZ79kZD_vrwHyd2WZzx-9FF3D6CVVi6X=Cx1=HAgr1gqNKyaA@mail.gmail.com>
- In-Reply-To
- <56731715.9000509@web.de>
On Thu, Dec 17, 2015 at 12:12 PM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 40 quoted lines
> 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?In case poll fails (assume a no op for it), the logic should not have changed by this patch?
Looking closely:
Show 20 quoted lines
>> 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);Or do you mean to insert another continue in here?
Show 7 quoted lines
>> + } >> + } >> return nr; >> } >> } >> >