Skip to content

Comments

Fix integer overflow on 16-bit MCUs in timeout calculations#2

Draft
Copilot wants to merge 5 commits intomasterfrom
copilot/fix-1
Draft

Fix integer overflow on 16-bit MCUs in timeout calculations#2
Copilot wants to merge 5 commits intomasterfrom
copilot/fix-1

Conversation

Copy link

Copilot AI commented Jun 9, 2025

This PR fixes an integer overflow issue that occurs when compiling for 16-bit MCUs like Arduino Uno, where int is only 16 bits.

Problem

The expression (90 * 1000) in the timeout calculation overflows on 16-bit systems because:

  • 90 × 1000 = 90,000
  • Maximum 16-bit signed int value = 32,767
  • This causes a compiler warning: 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:

// Before (overflows on 16-bit)
result = (90 * 1000);
result = (JGetInt(req, "seconds") * 1000);
timeoutMs = timeoutSeconds * 1000;

// After (safe on all platforms)  
result = (90U * 1000U);
result = (JGetInt(req, "seconds") * 1000U);
timeoutMs = timeoutSeconds * 1000U;

The U suffix 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

  • n_request.c: Updated 4 multiplication operations to use unsigned constants
  • test file: Updated test expectations for consistency

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.

Copilot AI and others added 3 commits June 9, 2025 21:17
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
Copilot AI requested a review from zakoverflow June 9, 2025 21:22
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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will overflow a 16-bit integer. You have failed the primary objective of this PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
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.

Integer overflow on 16-bit MCUs

2 participants