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

[Jeff King <peff@peff.net>]

On Fri, Dec 18, 2015 at 09:50:46AM +0100, Torsten Bögershausen wrote:

> >> 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.
> [...]
>
> /* So the code v2 would look like this: */
> 
>     if (!poll(&pfd, 1, -1)) {
>         if (errno == EINTR)
>             continue;
>          return -1; /* poll() failed, this is serious. */
>     }

That solves the EINTR problem, but I still don't see why we want to
return -1. The caller asked us to read(). We know that read() did not
fail with an actual error. Yet we are going to return an error to the
user, with errno set to something related only to poll(). I think we are
better off to keep the same semantics from the caller's point of view:
we loop until read() returns forward progress or a real error, and
anything else we do is a behind-the-scenes optimization.

BTW, I am assuming you mean:

  if (poll(&pfd, 1, -1) < 0)
	...

in your examples. Returning "0" means that poll timed out, but of course
we are not providing a timeout.

-Peff