print: Added size_t print format specifier #20194
Merged
maribu merged 8 commits intoRIOT-OS:masterfrom Dec 21, 2023
Merged
Conversation
fzi-haxel
requested review from
MichelRottleuthner,
PeterKietzmann,
aabadie,
benpicco,
bergzand,
cgundogan,
daniel-k,
dylad,
fjmolinas,
haukepetersen,
jia200x,
jue89,
kaspar030,
leandrolanzieri,
maribu,
miri64 and
smlng
as code owners
github-actions
bot
added
Area: network
benpicco
added
the
CI: ready for build
maribu
reviewed
Dec 19, 2023
Member
maribu
left a comment
maribu
left a comment
There was a problem hiding this comment.
Thx, this looks good to me. I have a number of nitpicks inline, though.
Contributor
Author
|
Thanks for the detailed review. |
OlegHahm
reviewed
Dec 19, 2023
Member
OlegHahm
left a comment
OlegHahm
left a comment
There was a problem hiding this comment.
I didn't look through everything and only found a few minor things/nitpicks.
maribu
reviewed
Dec 20, 2023
Contributor
Author
|
Should I squash the fixups and improve the commit messages a bit, or should I wait for more reviews? |
Member
|
Feel free to squash |
- Added `PRIuSIZE`, `PRIxSIZE`, etc. to `architecure.h` - Changed `CODING_CONVENTIONS.md` regarding `size_t` printing Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Marian Buschsieweke <[email protected]>
- Added helper function to avoid 'format-truncation' warning - Changed all `size_t` types to `unsigned` - Changed function names from `_FIB_` to `_fib_`
fzi-haxel
force-pushed
the
pr/size_t_print_modifier
branch
from
cdb9ba9 to
475a551
Compare
Contributor
Author
|
I squashed the fixups as well as the one review commit, and changed the commit messages a bit. |
maribu
approved these changes
Dec 21, 2023
maribu
added
Type: cleanup
fzi-haxel
added a commit
to fzi-haxel/RIOT
that referenced
this pull request
Jan 12, 2024
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
6 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.
Contribution description
As discussed in #20182, this PR adds custom print format specifiers for
size_t(e.g.,PRIuSIZE) that doesn't assumesizeof(unsigned) == sizeof(size_t)toarchitecture.hand changes thesize_tprinting recommendation inCODING_CONVENTIONS.mdFurthermore, the bulk of this PR consists of changes that modify print statements with
size_targuments to use the new print format specifiers.@maribu mentioned a possible switch to picolib in the near future. This would provide support for the currently problematic
%zformat specifier. If this is the preferred solution, I can easily replace thePRIuSIZE,PRIxSIZE, etc. macros with the corresponding%zmodifiers and remove the custom print format specifiers.Testing procedure
This shouldn't change any test or application.
To test the debug statement, set the appropriate
ENABLE_DEBUGmacro (or enable it globally).Issues/PRs references
See also #20182, #20154