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. */
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
