Fix overflow risk in strobeBlinkPin by updating parameter types and loop counter #519
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…prevent overflow Changed the loop counter and interval parameters in strobeBlinkPin to use unsigned integer types (unsigned int for count and uint32_t for on/off intervals) to avoid overflow and incorrect behavior when count exceeds 255 or intervals are large. This fixes potential infinite loops or unexpected delays caused by using byte (uint8_t) types for variables that can hold larger values.
…gned types Updated the strobeBlinkPin function prototype in the header file to use unsigned int for count and uint32_t for onInterval and offInterval to match the implementation and prevent overflow issues. This ensures type consistency and avoids potential bugs caused by signed or too-small integer types for timing and loop count parameters.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a potential overflow issue in the
strobeBlinkPinfunction in the Firmata library by changing the types of the parameters and the loop counter to unsigned integer types.Changes made
countparameter frominttounsigned intto reflect that it cannot be negative.onIntervalandoffIntervalfrominttouint32_tto support longer delay durations and prevent negative values.bytetounsigned intfor correct loop iteration whencountis greater than 255..cppand.hfiles for consistency.Why this is important
Previously, using a
bytefor the loop counter could cause overflow and incorrect behavior whencountexceeds 255, potentially leading to infinite loops or premature termination. Similarly, using signed integers for timing parameters could allow negative values, causing unexpected delays.This fix ensures type safety, correct loop behavior, and better support for longer timing intervals.
Additional notes
Please review and let me know if you'd like to discuss or improve the approach further.