Fix integer overflow on 16-bit MCUs in timeout calculations#2
Draft
Fix integer overflow on 16-bit MCUs in timeout calculations#2
Conversation
Co-authored-by: zakoverflow <215570245+zakoverflow@users.noreply.github.com>
Co-authored-by: zakoverflow <215570245+zakoverflow@users.noreply.github.com>
Co-authored-by: zakoverflow <215570245+zakoverflow@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Integer overflow on 16-bit MCUs
Fix integer overflow on 16-bit MCUs in timeout calculations
Jun 9, 2025
zakoverflow
reviewed
Jun 9, 2025
n_request.c
Outdated
| NOTE_C_LOG_DEBUG("No `milliseconds` or `seconds` parameter " | ||
| "provided. Defaulting to 90-second timeout."); | ||
| result = (90 * 1000); | ||
| result = (90U * 1000U); |
Owner
There was a problem hiding this comment.
This will overflow a 16-bit integer. You have failed the primary objective of this PR.
Author
There was a problem hiding this comment.
Fixed the integer overflow by changing (90U * 1000U) to (90UL * 1000UL). The issue was that 90U * 1000U uses unsigned int arithmetic, which on 16-bit systems is still 16-bit (max 65,535), and 90 × 1000 = 90,000 overflows that. Using UL forces 32-bit unsigned long arithmetic. Also updated all other timeout multiplications for consistency. (commit a61c3b8)
…lications Co-authored-by: zakoverflow <215570245+zakoverflow@users.noreply.github.com>
zakoverflow
approved these changes
Jun 9, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR fixes an integer overflow issue that occurs when compiling for 16-bit MCUs like Arduino Uno, where
intis only 16 bits.Problem
The expression
(90 * 1000)in the timeout calculation overflows on 16-bit systems because:integer overflow in expression [-Woverflow]On 16-bit systems, the overflow would produce an incorrect timeout value of 24,464 ms instead of the expected 90,000 ms.
Solution
Changed all timeout-related multiplication operations to use unsigned arithmetic:
The
Usuffix ensures the multiplication is performed in unsigned arithmetic, which is guaranteed to be at least 32-bit, preventing overflow while maintaining identical functional behavior.Changes
All changes are minimal and preserve existing functionality while eliminating the compiler warning on 16-bit MCUs.
Fixes #1.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.