Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: add cbprintf capability #29876

Merged
merged 11 commits into from
Nov 13, 2020
Merged

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Nov 9, 2020

This PR adds a C99 stdio value formatter capability named cbprintf() (plus related variants) where generated text is emitted through a callback. This allows generation of arbitrarily long output without a buffer, functionality that is core to printk, logging, and other system and application needs.

The formatter supports most C99 specifications, excluding:

  • %L[aefg] long double conversion
  • wide character output

The new solution is derived from z_prf(), which had the most complete functionality available. Format parsing has been reimplemented, while floating point conversion has been retained and enhanced.

Kconfig options allow disabling features like floating-point conversion if they are not necessary.

Benefits include:

  • No more inconsistencies between printk, logging, and shell formatting capabilities.
  • Reduced inconsistencies due to toolchain libc capabilities.
  • Simplified (toolchain-independent) formatting feature selection via Kconfig.
  • Core infrastructure can avoid duplication with libc printf by using cbprintf variants.
  • Potentially a path to resolving logging: 32 bit float values don't work. #18351 and related issues, as the infrastructure can tell a caller the total size of arguments to the conversion string.
  • Fixed bugs with several specifications.

Costs include:

  • Applications that used only printk with no extra features consume more code space than the previous implementation.

Fixes #29202

@github-actions github-actions bot added area: API Changes to public APIs area: C Library C Standard Library area: Logging area: Shell Shell subsystem area: Tests Issues related to a particular existing or missing test labels Nov 9, 2020
@pabigot pabigot force-pushed the nordic/20201108a branch 3 times, most recently from 34da0c2 to 0238350 Compare November 9, 2020 16:48
@pabigot pabigot marked this pull request as ready for review November 9, 2020 18:12
@pabigot pabigot requested a review from npitre November 9, 2020 18:14
Copy link
Member

@nashif nashif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this is similar to what I was trying to do with just the moving things around :)

https://github.com/zephyrproject-rtos/zephyr/compare/master...nashif:zprf_vprintk?expand=1

Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great.
I'm assuming stack usage for printk() is roughly like it was before? Passing CI result suggests to me that it does, but I thought I'd ask.

Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great.
I'm assuming stack usage for printk() is roughly like it was before? Passing CI result suggests to me that it does, but I thought I'd ask.

Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great.
I'm assuming stack usage for printk() is roughly like it was before? Passing CI result suggests to me that it does, but I thought I'd ask.

Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great.
I'm assuming stack usage for printk() is roughly like it was before? Passing CI result suggests to me that it does, but I thought I'd ask.

@pabigot
Copy link
Collaborator Author

pabigot commented Nov 9, 2020

this looks great.

Thanks!

I'm assuming stack usage for printk() is roughly like it was before?

I expect it's a little larger compared to z_vprintk, but smaller for z_prf(), depending on how much the compiler could overlay the local variables. I don't know of any easy way to get real data.

@npitre
Copy link
Collaborator

npitre commented Nov 9, 2020 via email

petejohanson added a commit to petejohanson/zmk that referenced this pull request Jun 10, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
petejohanson added a commit to petejohanson/zmk that referenced this pull request Jun 10, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
petejohanson added a commit to petejohanson/zmk that referenced this pull request Jun 17, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
petejohanson added a commit to petejohanson/zmk that referenced this pull request Jun 18, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
petejohanson added a commit to petejohanson/zmk that referenced this pull request Jun 18, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
innovaker pushed a commit to innovaker/zmk that referenced this pull request Jun 19, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
innovaker pushed a commit to innovaker/zmk that referenced this pull request Jun 19, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
innovaker pushed a commit to innovaker/zmk that referenced this pull request Jun 19, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
innovaker pushed a commit to innovaker/zmk that referenced this pull request Jun 19, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
petejohanson added a commit to petejohanson/zmk that referenced this pull request Jul 15, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
petejohanson added a commit to zmkfirmware/zmk that referenced this pull request Jul 17, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: #736
crides pushed a commit to crides/zmk that referenced this pull request Nov 3, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
fsargent pushed a commit to fsargent/zmk that referenced this pull request Nov 19, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
Luberry pushed a commit to Luberry/zmk that referenced this pull request Nov 30, 2021
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
PedroDiogo pushed a commit to PedroDiogo/zmk that referenced this pull request Feb 7, 2022
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
bromanko pushed a commit to bromanko/zmk that referenced this pull request Feb 25, 2022
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
stephanosio added a commit to stephanosio/zephyr that referenced this pull request May 26, 2022
This commit adds the missing `PRIxMAX` macros for the C99 `intmax_t`
and `uintmax_t` types:

  PRIdMAX, PRIiMAX, PRIoMAX, PRIuMAX, PRIxMAX, PRIXMAX

Note that the `PRIxMAX` macros specify the `ll` size modifier because
the type of the `intmax_t` for the minimal libc is defined as that of
the `int64_t`, which is always overridden to `long long int` by
`zephyr_stdint.h`; for more details, refer to the GitHub PR zephyrproject-rtos#29876,
which deliberately introduced this scheme.

In the future, this scheme will need to be reworked such that the
minimal libc `stdint.h` defines `intmax_t` as `__INTMAX_TYPE__`, and
the `inttypes.h` resolves the corresponding format specifier.

Signed-off-by: Stephanos Ioannidis <[email protected]>
carlescufi pushed a commit that referenced this pull request May 26, 2022
This commit adds the missing `PRIxMAX` macros for the C99 `intmax_t`
and `uintmax_t` types:

  PRIdMAX, PRIiMAX, PRIoMAX, PRIuMAX, PRIxMAX, PRIXMAX

Note that the `PRIxMAX` macros specify the `ll` size modifier because
the type of the `intmax_t` for the minimal libc is defined as that of
the `int64_t`, which is always overridden to `long long int` by
`zephyr_stdint.h`; for more details, refer to the GitHub PR #29876,
which deliberately introduced this scheme.

In the future, this scheme will need to be reworked such that the
minimal libc `stdint.h` defines `intmax_t` as `__INTMAX_TYPE__`, and
the `inttypes.h` resolves the corresponding format specifier.

Signed-off-by: Stephanos Ioannidis <[email protected]>
laxiLang pushed a commit to laxiLang/zephyr that referenced this pull request May 30, 2022
This commit adds the missing `PRIxMAX` macros for the C99 `intmax_t`
and `uintmax_t` types:

  PRIdMAX, PRIiMAX, PRIoMAX, PRIuMAX, PRIxMAX, PRIXMAX

Note that the `PRIxMAX` macros specify the `ll` size modifier because
the type of the `intmax_t` for the minimal libc is defined as that of
the `int64_t`, which is always overridden to `long long int` by
`zephyr_stdint.h`; for more details, refer to the GitHub PR zephyrproject-rtos#29876,
which deliberately introduced this scheme.

In the future, this scheme will need to be reworked such that the
minimal libc `stdint.h` defines `intmax_t` as `__INTMAX_TYPE__`, and
the `inttypes.h` resolves the corresponding format specifier.

Signed-off-by: Stephanos Ioannidis <[email protected]>
tyalie pushed a commit to tyalie/zmk that referenced this pull request Nov 15, 2022
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
NikolaRavic pushed a commit to NikolaRavic/zmk-firmware that referenced this pull request Feb 27, 2023
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
yuanbin pushed a commit to yuanbin/zmk that referenced this pull request Jun 14, 2023
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
infused-kim pushed a commit to infused-kim/kb_zmk_ps2_mouse_trackpoint_driver that referenced this pull request Apr 14, 2024
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware/zmk#736
think4tomorrow added a commit to think4tomorrow/zhc that referenced this pull request Dec 18, 2024
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware/zmk#736
zoowmooz pushed a commit to zoowmooz/zmk that referenced this pull request Feb 2, 2025
* new cbprintf formatter causes issues for our use of string formatting.

See: zephyrproject-rtos/zephyr#29876
PR: zmkfirmware#736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: C Library C Standard Library area: Documentation area: Kernel area: Logging area: Shell Shell subsystem area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core kernel depends on minimal libc z_prf()
6 participants