perf: drop <sstream> from JsonSettings, freeing 256KB of flash - #47
Open
socquique wants to merge 1 commit into
Open
perf: drop <sstream> from JsonSettings, freeing 256KB of flash#47socquique wants to merge 1 commit into
socquique wants to merge 1 commit into
Conversation
getIntVector()/putIntVector() used std::istringstream/ostringstream to handle a comma separated list of small integers. Including <sstream> drags in the C++ iostreams and locale machinery, and with it newlib's wide-character and floating point printf/scanf families - _svfwprintf_r, __ssvfscanf_r, _vfprintf_r, _dtoa_r and friends. Hand-rolled strtol parsing removes all of it: 254,816 bytes of flash, taking the esp32_c3 image from 99.2% to 79.7% of its partition. Parsing is equivalent for well formed input and stops at the first non-numeric token instead of throwing, so it no longer needs std::stoi's exceptions either. That leniency exposed an existing hole: SplitFlapDisplay::init() sized its loops from moduleCount but indexed the address and offset vectors without checking their length, reading past the end for a short or malformed list. Both now fall back to per-module defaults, and numModules is clamped to MAX_MODULES so it cannot overrun modules[]. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Problem
JsonSettings::getIntVector()/putIntVector()usedstd::istringstreamandstd::ostringstreamto handle a comma separated list of small integers.Including
<sstream>pulls the C++ iostreams andstd::localemachinery into theimage, and with it newlib's wide-character and floating point printf/scanf
families. The largest symbols actually linked into the
esp32_c3build:None of that is used by this project.
_svfwprintf_ris wide-character printf.Change
Hand-rolled
strtolparsing andStringconcatenation instead.That is the difference between an image with 11 KB of headroom and one with 268 KB.
The
esp32_c3build was close enough to its partition that adding almost anythingwould have overflowed it.
Parsing is equivalent for well formed input; it stops at the first non-numeric
token instead of throwing, so it no longer needs
std::stoi's exceptions.Bounds check
That leniency exposes an existing hole, fixed in the same commit:
SplitFlapDisplay::init()sizes its loops frommoduleCountbut indexed theaddress and offset vectors without checking their length, reading past the end of
the vector for a short or malformed list. Both now fall back to per-module
defaults, and
numModulesis clamped toMAX_MODULESso it cannot overrunmodules[].Testing
Parser checked against the real setting values and edge cases — well formed lists,
embedded spaces, trailing comma, empty string, single value. Built and run on an
8 module ESP32-C3 display; addresses and offsets load identically.
Touches
JsonSettings.cppalongside the settings PR; trivial to rebase whicheverlands second.