Repository navigation
Make settings.toml numbers follow TOML and use less flash - #11620
Merged
Merged
Conversation
TOML does not allow leading zeros on decimal integers or on the integer part of floats (https://toml.io/en/v1.0.0#integer). strtol base 0 read 017 as octal 15, and values like 08 or 01.5 fell through to the float parser. They now return SETTINGS_ERR_BAD_VALUE like other invalid numbers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
dhalbert
requested changes
Oct 16, 2026
dhalbert
left a comment
dhalbert
left a comment
Collaborator
There was a problem hiding this comment.
Thanks - couple of minor points.
TOML allows leading zeros after the 0x prefix, so check that 0x000F still parses as 15. Note in environment.rst that TOML allows 0o, 0b and underscores, and only this parser does not. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
TOML allows underscores between digits in integers and floats, such as 1_000 or 0x1234_abcd. Each underscore must have a digit on both sides. Before, strtol() stopped at the underscore, so 1_000 fell through to the float parser and came back as 1000.0. mp_parse_float_internal() skips any underscore, so 1__0, _1 and 1_ were also accepted. Remove underscores in place after checking each one, before parsing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
dhalbert
requested changes
Oct 16, 2026
dhalbert
left a comment
dhalbert
left a comment
Collaborator
There was a problem hiding this comment.
What provoked this PR in the first place? Was there some case where someone had leading zeros and it was interpreted as octal?
Collaborator
Author
|
This was part of the test fixture design for CircuitPython. A hardware test that checks settings.toml against values TOML rejects found that It seemed worth fixing before anyone hit it. |
This reverts commit 50ee27e. The extra code is not worth it: we do not need to match the TOML underscore rules exactly. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Only a digit after the leading 0 needs to be rejected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Replace strtol() in get_int() and strtoul() in read_unicode_escape() with a small digit parser. This drops strtol, strtoul, _ctype_, errno and _impure_data from the link: -660 bytes flash and -80 bytes RAM on the ItsyBitsy nRF52840, -620 bytes flash on the Feather RP2040. Integers now also accept 0o and 0b prefixes and underscores (1_000). As a superset of TOML, a sign is allowed before 0x/0o/0b, the prefix may be uppercase, and underscores are not checked to be between digits. Out-of-range integers are no longer clamped to LONG_MAX. settings_get_int() returns an error for them; supervisor.get_setting() raises ValueError for hex, octal and binary, and returns a float for decimal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
'@' and '`' were read as the digit 9, so 12@ parsed as 129. Stop at any byte below 'a' after case folding. Underscores were skipped inside \u escapes, so "\u0_41" decoded to "A". Read escape digits directly instead of through parse_digits(). This also drops a variable-length array. Fold the decimal leading-zero check into the existing prefix check and move has_leading_zero() under MICROPY_PY_BUILTINS_FLOAT, since only get_float() uses it now. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
dhalbert
approved these changes
Oct 18, 2026
dhalbert
left a comment
dhalbert
left a comment
Collaborator
There was a problem hiding this comment.
Great, smaller size and better checking.
mikeysklar
changed the title
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
2 participants
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.
What
supervisor.get_setting()now rejects numbers with leading zeros and reads integers with a small built-in parser.Why
017was read as octal 15, which TOML does not allow.Dropping
strtol()also saves flash.017,-017,0015,-15,0ValueError08,01.5,007e18.0,1.5,70.0ValueError0o17,0b101ValueError15,51_0001000.010000xFFFFFFFF2147483647ValueError999999999992147483647100000000000.0"\u+041"'A'ValueError0,-17,+17,0x11,0X1f,0.5,1e3Bad startup settings print "Invalid format" and keep their default.
An oversized
CIRCUITPY_WEB_API_PORTnow uses port 80 instead of 65535.The integer parser is from @dhalbert.
.bin)strtoul()tests/circuitpython-manual/settingson Metro RP2040, Feather nRF52840, Metro ESP32-S3make testOut of scope:
0_17and1_are accepted,\uD800surrogates are accepted, binary values longer than 13 digits do not fit the buffer.Written with Claude Code, which also ran the tests above. I reviewed the code and this description.
🤖 Generated with Claude Code