Repository navigation
Format units longer than 255 bytes in full - #444
Merged
Merged
Conversation
R_ut_format() formatted into a 256-byte buffer and warned only when ut_format() returned exactly sizeof(buf). ut_format() truncates to fit and returns the length it needs, so any unit whose string form is longer than 255 bytes came back silently cut short, and callers re-parsed the stub. For a product of about 52 or more user-defined base units, convert_to_base() then failed with "syntax error, cannot parse '...'" or "cannot convert ... into ...". When the buffer is too small, format again into one of the length ut_format() reports. Also stop on a negative return instead of returning an unset buffer (unreachable while r_error_fn throws first). Tests cover formatted lengths of exactly 255, 256 and 257 bytes, and 300, through ud_parse() and through convert_to_base(). Co-Authored-By: Claude Opus 5.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.
Summary
R_ut_format()formatted units into a 256-byte buffer and warned only whenut_format()returned exactlysizeof(buf). Butut_format()truncates to fit and returns the length it needs. Any unit whose string form was longer than 255 bytes therefore came back silently cut short, and callers re-parsed the truncated string. For a product of about 52 or more user-defined base units,convert_to_base()then failed withsyntax error, cannot parse '...'orcannot convert ... into ....Fix
ut_format()reports.r_error_fnthrows first.== sizeof(buf)warning.Tests
ud_parse()andconvert_to_base(). They fail on main and pass with the fix.R CMD check: Status OK; tests 596 pass, 0 fail.Relationship to #443
Both PRs touch the formatting helper, so whichever lands second merges the other. The conflicts, in
src/udunits.cpp,NEWS.mdandtests/testthat/test_udunits.R, are mechanical:R_ut_format()and move this retry into itsformat_unit();}).A trial merge gives 594 expectations and 0 failures.
🤖 Generated with Claude Code