Skip to content

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

New issue

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

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

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants