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

Revisit: mbedtls port: Fix detection of EWOULDBLOCK/EAGAIN with non-blocking sockets #511

Closed
wants to merge 5 commits into from

Conversation

mkellner
Copy link
Contributor

Since mbedtls_net_errno is reset by fcntl, it is reset after calling
net_would_block, so the call to mbedtls_net_errno in mbedtls_net_recv
and mbedtls_net_send will always get back 0. This change propagates
the value returned by mbedtls_net_errno up through net_would_block,
to allow the correct error value to be used and avoid a redundant
call to mbedtls_net_errno.

net_would_block, so the call to mbedtls_net_errno in mbedtls_net_recv
and mbedtls_net_send will always get back 0. This change propagates
the value returned by mbedtls_net_errno up through net_would_block,
to allow the correct error value to be used and avoid a redundant
call to mbedtls_net_errno.
Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

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

This looks great, thank you. I like your fix.

This will also close #434 once merged.

Have requested one minor change but otherwise is great.

{
int error = mbedtls_net_errno(ctx->fd);

if ( errout ) *errout = error;
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor style thing, but can you please put the assignment statement on a separate line inside curly braces. This in keeping with conditional style in the rest of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. Done.

…bits. Also, there are separate lengths for rx and tx (confusingly named rxlength and length... if rxlength is 0, length is used). The code checks the tx length for the rx, so it never validates rxlength.
@projectgus
Copy link
Contributor

Thanks Michael. Sorry it took me so long to get back to this. I've squashed the mbedTLS commits and pushed them into our internal review & merge queue.

I've also added the SPI master driver fix d92d57c.

Angus

@projectgus projectgus added the Status: Pending blocked by some other factor label Apr 21, 2017
igrr pushed a commit that referenced this pull request Apr 21, 2017
…ockets

Since mbedtls_net_errno is reset by fcntl, it is reset after calling
net_would_block, so the call to mbedtls_net_errno in mbedtls_net_recv
and mbedtls_net_send will always get back 0. This change propagates
the value returned by mbedtls_net_errno up through net_would_block,
to allow the correct error value to be used and avoid a redundant
call to mbedtls_net_errno.

Merges PR #511 #511
igrr pushed a commit that referenced this pull request Apr 21, 2017
SPI transfer length is bits, not bytes, so the error should indicate bits. Also, there are separate lengths for rx and
tx (confusingly named rxlength and length... if rxlength is 0, length is used). The code checks the tx length for the
rx, so it never validates rxlength.

Originally contributed as part of #511 #511
igrr pushed a commit that referenced this pull request Apr 21, 2017
mbedtls port: Fix detection of EWOULDBLOCK/EAGAIN with non-blocking sockets

Since mbedtls_net_errno is reset by fcntl, it is reset after calling
net_would_block, so the call to mbedtls_net_errno in mbedtls_net_recv
and mbedtls_net_send will always get back 0. This change propagates
the value returned by mbedtls_net_errno up through net_would_block,
to allow the correct error value to be used and avoid a redundant
call to mbedtls_net_errno.

Merges PR #511 #511

See merge request !688
@projectgus
Copy link
Contributor

Cherry-picked as a523aa3 and eeb0aaa, thanks!

@projectgus projectgus closed this Apr 25, 2017
@igrr igrr removed the Status: Pending blocked by some other factor label Nov 14, 2017
turmary pushed a commit to Seeed-Studio/Seeed_Arduino_mbedtls that referenced this pull request Jan 22, 2020
…ockets

Since mbedtls_net_errno is reset by fcntl, it is reset after calling
net_would_block, so the call to mbedtls_net_errno in mbedtls_net_recv
and mbedtls_net_send will always get back 0. This change propagates
the value returned by mbedtls_net_errno up through net_would_block,
to allow the correct error value to be used and avoid a redundant
call to mbedtls_net_errno.

Merges PR #511 espressif/esp-idf#511
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants