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

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

On 18.12.15 04:13, Jeff King wrote:
> On Thu, Dec 17, 2015 at 09:42:01PM +0100, Torsten Bögershausen wrote:
> 
>>> Or do you mean to insert another continue in here?
>> I was thinking that we run into similar loop as before:
>> read() returns -1; errno = EAGAIN  /* No data to read */
>> poll() returns -1; errno = EAGAIN /* poll failed. If the fd was OK, the failure may be temporaly,
>>                                     as much as poll() can see.
>>                                     But most probably we run out ouf memory */
>>
>> So the code would look like this:
>>
>>    if (!poll(&pfd, 1, -1))
>>       return -1;
> 
> That changes the semantics of the function. The poll() is just a
> convenience to avoid spinning. If it fails, with Stefan's patch[1] the
> worst case is that we would spin on read() and poll(), instead of
> actually blocking in the poll().
> 
> But if we return on poll() failure, now the caller will see errors from
> poll() even though they don't know or care that we called poll() in the
> first place. Consider what would happen with your code if read got
> EAGAIN and then poll got EINTR. We would report an error, even though
> the whole point of xread() is to loop on these conditions.
> 
> -Peff
> 
> [1] Stefan's patch does have a bug. It forgets to "continue" after
>     calling poll, which means we will block with poll and _then_ exit
>     with -1, instead of restarting the loop.
> --
I love this group: Curing one bug with another doesn't work :-)

/* So the code v2 would look like this: */

    if (!poll(&pfd, 1, -1)) {
        if (errno == EINTR)
            continue;
         return -1; /* poll() failed, this is serious. */
    }