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

Fix count overflow on readBytes/readWords #750

Merged
merged 1 commit into from
May 28, 2023

Conversation

nekomona
Copy link
Contributor

No description provided.

@nekomona
Copy link
Contributor Author

Detailed description in #751.
The patch fixed the issue in Arduino lib. For other MCUs it may need further verification.

@jrowberg
Copy link
Owner

Good catch on this...I think most people must not have tried to read that many bytes all at once. The solution here is an improvement over what was previously implemented, but it still leaves ambiguity about what the return code means when the raw value is above +127. If you read 168 bytes successfully, but compare the function return value with a simple result < 0 kind of check, you'll incorrectly detect an error because the function's actual return type is still int8_t. Maybe the cleaner fix is to leave the max read length as a uint8_t but change the count and return type to int16_t.

@jrowberg jrowberg merged commit 605a740 into jrowberg:master May 28, 2023
@nekomona
Copy link
Contributor Author

Changing return type should be a clean fix for the library but I'm not sure if it would hit existing programs with return values stored in a int8_t variable. Guess I'll leave the issue open to mark that.

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.

2 participants