Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: sockets: Handle EINTR return from k_poll() #5024

Merged
merged 1 commit into from
Nov 21, 2017

Conversation

pfalcon
Copy link
Contributor

@pfalcon pfalcon commented Nov 16, 2017

In 90b471f, there was a change to make k_poll() return EINTR error
if it was cancelled with k_fifo_cancel_wait(). Handle this change, or
otherwise sockets EOF handling was broken.

Signed-off-by: Paul Sokolovsky paul.sokolovsky@linaro.org

In 90b471f, there was a change to make k_poll() return EINTR error
if it was cancelled with k_fifo_cancel_wait(). Handle this change, or
otherwise sockets EOF handling was broken.

Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
@@ -298,7 +298,8 @@ static inline ssize_t zsock_recv_stream(struct net_context *ctx,
}

res = _k_fifo_wait_non_empty(&ctx->recv_q, timeout);
if (res && res != -EAGAIN) {
/* EAGAIN when timeout expired, EINTR when cancelled */
if (res && res != -EAGAIN && res != -EINTR) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what circumstances is this condition true?
I'm looking at the k_poll() implementation and it seems that it will only return 0, -EAGAIN, or -EINTR.

side note: the referenced patch did not update the doxygen for k_poll() with the new return value semantics, filing a bug on that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at the k_poll() implementation and it seems that it will only return 0, -EAGAIN, or -EINTR.

And few months ago, it (supposedly) could only return 0, -EAGAIN. Relying on things like that would be possible only if k_poll docs explicitly mentioned that it can return only those values. And even then I'd probably prefer to re-insure, because it's not yet time to make such API contracts as k_poll() is in flux. (And I'm writing that as a guy who don't like superfluous checks, and can easily suggest to remove some in other folks' reviews.)

@pfalcon
Copy link
Contributor Author

pfalcon commented Nov 20, 2017

@jukkar : Thought you're among the reviewers, please review.

Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pfalcon pfalcon requested a review from askawu November 21, 2017 07:25
@nashif nashif merged commit 21f31e9 into zephyrproject-rtos:master Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants