Skip to content

Fix WiFiClientRxBuffer::fillBuffer() error, likely solves issue #2212 #2259

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

Closed
wants to merge 3 commits into from
Closed

Fix WiFiClientRxBuffer::fillBuffer() error, likely solves issue #2212 #2259

wants to merge 3 commits into from

Conversation

Jeroen88
Copy link
Contributor

@Jeroen88 Jeroen88 commented Jan 1, 2019

Fix WiFiClientRxBuffer::fillBuffer() error in handing a negative res from the call to recv()

In

if(res < 0 && errno != EWOULDBLOCK) {
if res is negative and errno equals EWOULDBLOCK then the if() condition is false causing _fill to be decremented.
The condition res < 0 && errno == EWOULDBLOCK simply means that the socket has no data available (yet), so _fill should not change and, since nothing is read, the return value of fillBuffer should be zero.

This most likely solves the complete #2212 issue, at least it solves the minimum sketch reproducing the error as posted in that issue by @lbernstone. This could also be the root cause of other issues, since on slow connections it is likely that the socket not always has data available.

I also added an extra test on

_buffer = (uint8_t *)malloc(_size);
by setting _failed to true if the malloc() fails. I am not sure and did not check if the _failed condition is checked in the other functions, but it should not do any harm. Moreover, this did not cause the error.

Happy New Year to everyone!

@Jeroen88 Jeroen88 mentioned this pull request Jan 1, 2019
@Jeroen88
Copy link
Contributor Author

Jeroen88 commented Jan 1, 2019

@lbernstone, @nikuz confirmed that my PR fixes the error. Also, the if() in WiFiClientRxBuffer::fillBuffer() before my PR is obviously wrong...

@lbernstone
Copy link
Contributor

lbernstone commented Jan 1, 2019

Verified. Looks like you included some BLE stuff in your commit, tho.
Fixes #2212

@Jeroen88
Copy link
Contributor Author

Jeroen88 commented Jan 1, 2019

@lbernstone You are right, I always pull remote master before creating a branch. So I was surpised to have some BLE stuff in my local master

@me-no-dev
Copy link
Member

I can not merge this with the BLE stuff in there ;)

@Jeroen88
Copy link
Contributor Author

Jeroen88 commented Jan 2, 2019

@me-no-dev can you help me get it out? I did a git pull upstream master and then it got in...????

@me-no-dev
Copy link
Member

delete the BLE folder then call git submodule update --init --recursive in the root arduino folder

@Jeroen88
Copy link
Contributor Author

Jeroen88 commented Jan 2, 2019

Replaced by #2263

@Jeroen88 Jeroen88 closed this Jan 2, 2019
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.

4 participants