Skip to content

Comments

Fix warning regarding possible buffer overrun#17513

Merged
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-2.0.xfrom
sjasonsmith:17506_sprintf_warning
Apr 20, 2020
Merged

Fix warning regarding possible buffer overrun#17513
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-2.0.xfrom
sjasonsmith:17506_sprintf_warning

Conversation

@sjasonsmith
Copy link
Contributor

Description

Fix a warning indicating the sprintf command could overrun the destination buffer.

'%i' directive writing between 1 and 6 bytes into a region of size between 5 and 10 [-Wformat-overflow=]

The solution was two-fold:

  • Make temp an int16_t to reduce the maximum printed length (this matches the source data size)
  • Use C99-style format specifier to ensure the format specifier matches the argument length on all platforms.

Related Issues

#17506

@tpruvot
Copy link
Contributor

tpruvot commented Apr 15, 2020

Good on the F1, removes the warning, was introduced by #17459
Another solution was to extend the 24 bytes buffer to 28 bytes

@thinkyhead
Copy link
Member

I wonder if it would be beneficial to apply the same format specifier elsewhere…

@thinkyhead thinkyhead merged commit cba58b1 into MarlinFirmware:bugfix-2.0.x Apr 20, 2020
@sjasonsmith
Copy link
Contributor Author

I wonder if it would be beneficial to apply the same format specifier elsewhere…

Fortunately newer versions of gcc are a lot better about warning you when they will cause issues. At least on intel architectures you could have bad behaviors if the format specifier and argument mismatched, because if arguments had to be passed on the stack it would pop the wrong number of bytes from the stack at the end of the call.

@sjasonsmith sjasonsmith deleted the 17506_sprintf_warning branch May 10, 2020 05:17
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
njibhu pushed a commit to njibhu/Marlin that referenced this pull request Aug 24, 2020
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants