Skip to content

pkg/mpaland-printf: Add alternative stdio as package #20664

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

Merged
merged 2 commits into from
Apr 25, 2025

Conversation

maribu
Copy link
Member

@maribu maribu commented May 11, 2024

Contribution description

This packs the stdio implementation from https://github.com/mpaland/printf as alternative to what the used standard C lib provides with the intent to provide a thread-safe, smaller, and more feature-complete alternative on newlib targets.

Compared to newlib_nano this reduces .text by a bit more than 200 B while adding support for standard format specifiers such as RPIu8, PRIu16, PRIu64, %z, and %t.

Note that newlib_nano's stdio can be thread-safe in reentrant mode at the cost of RAM (per thread) and latency. Especially the increase in latency can be prohibitive when real time requirements need to be met.

Testing procedure

A test application has been added in tests/sys/snprintf. This will use the package if newlib is used. It should pass, but removing Makefile.board.dep (which pulls in the mpaland-printf package) should a failing test.

Issues/PRs references

#20361

@maribu maribu added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 11, 2024
@github-actions github-actions bot added Area: doc Area: Documentation Area: build system Area: Build system labels May 11, 2024
@riot-ci
Copy link

riot-ci commented May 11, 2024

Murdock results

✔️ PASSED

42dacd7 tests/sys/snprintf: Test format specifiers

Success Failures Total Runtime
10320 0 10320 13m:26s

Artifacts

@maribu maribu force-pushed the pkg/mpaland-printf branch from 11a1309 to 817c8ff Compare May 11, 2024 20:49
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label May 11, 2024
@maribu maribu force-pushed the pkg/mpaland-printf branch from 817c8ff to 27a620b Compare May 11, 2024 20:53
@maribu maribu marked this pull request as ready for review May 11, 2024 20:56
@maribu maribu force-pushed the pkg/mpaland-printf branch 2 times, most recently from cf84d13 to a8ffaeb Compare May 12, 2024 11:23
@Enoch247 Enoch247 self-assigned this Apr 10, 2025
@Enoch247
Copy link
Contributor

I'll try and take a look at this later since I am familiar with the pkg already.

@Enoch247
Copy link
Contributor

Started looking this over, but ran out of time. I'll pick it up another night.

@Enoch247
Copy link
Contributor

Enoch247 commented Apr 12, 2025

For me, the new test would not build when doing make BOARD=stm32f429i-disc1 until I added the following hack:

diff --git a/tests/sys/snprintf/main.c b/tests/sys/snprintf/main.c
index 151966c0d5..0d27dca3cb 100644
--- a/tests/sys/snprintf/main.c
+++ b/tests/sys/snprintf/main.c
@@ -19,6 +19,7 @@
  * @}
  */
 
+#define __int64_t_defined 1
 #include <inttypes.h>
 #include <stdbool.h>
 #include <stddef.h>

@maribu
Copy link
Member Author

maribu commented Apr 12, 2025

Thx for reporting. I can reproduce the issue on my Ubuntu machine.

Can you give it another try with the fixup commit?

@Enoch247
Copy link
Contributor

Indeed make -C tests/sys/snprintf BOARD=stm32f429i-disc1 test passes if I use this branch unmodified, and fails if I disable mpaland-printf. Is newlib's snprintf implementation really broken? A quick internet search did not turn up anything about this. The test also passes on native where neither newlib nor mpaland-printf are enabled.

@maribu
Copy link
Member Author

maribu commented Apr 18, 2025

The newlib's stdio comes in two flavors: "super bloated" (stdio-nano), and "supermassive black hole of bloat" (regular stdio). The latter would pass the stido unit tests (try disabled newlib_nano), if it would even fit your MCU's flash and RAM. The former is a stripped down version of the latter, trading features (in other words compatibility with format specifiers) for size.

Both are way too large to be a sane option on MCUs. That is why the picolibs fork of newlib just trashed the stdio and replaces it with something that does make sense on MCUs.

@maribu maribu force-pushed the pkg/mpaland-printf branch from 848fa62 to 16dc151 Compare April 22, 2025 09:54
@Enoch247 Enoch247 added this pull request to the merge queue Apr 24, 2025
@maribu maribu removed this pull request from the merge queue due to the queue being cleared Apr 24, 2025
@maribu maribu added this pull request to the merge queue Apr 24, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 24, 2025
@maribu maribu force-pushed the pkg/mpaland-printf branch from 16dc151 to 557ff9e Compare April 24, 2025 09:38
@maribu maribu enabled auto-merge April 24, 2025 09:38
maribu added 2 commits April 24, 2025 11:46
This packs the stdio implementation from [1] as alternative to
what the used standard C lib provides with the intent to provide a
thread-safe, smaller, and more feature-complete alternative on
newlib targets.

Compared to `newlib_nano` this reduces `.text` by a bit more than 200 B
while adding support for standard format specifiers such as `RPIu8`,
`PRIu16`, `PRIu64`, `%z`, and `%t`.

Note that `newlib_nano`'s stdio can be thread-safe in reentrant mode
at the cost of RAM (per thread) and latency. Especially the increase
in latency can be prohibitive when real time requirements need to be
met.

[1]: https://github.com/mpaland/printf
This adds a simple test applications that runs snprintf on standard
format specifiers and compares the output with the expected output.

The assumption is that internally every stdio implementation uses the
same formatting code for each member of the printf functions family,
so testing snprintf only is sufficient.
@maribu maribu force-pushed the pkg/mpaland-printf branch from 533fa86 to 42dacd7 Compare April 24, 2025 09:46
@maribu maribu requested a review from jia200x as a code owner April 24, 2025 09:46
@maribu maribu added this pull request to the merge queue Apr 24, 2025
Copy link
Contributor

@mcr mcr left a comment

Choose a reason for hiding this comment

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

Having patches in git is giving me Quilt-PTSD, but I get why. I just hate it.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 24, 2025
@crasbe crasbe added this pull request to the merge queue Apr 24, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 24, 2025
@crasbe crasbe added this pull request to the merge queue Apr 24, 2025
Merged via the queue into RIOT-OS:master with commit d44d84d Apr 25, 2025
27 checks passed
@maribu maribu deleted the pkg/mpaland-printf branch April 25, 2025 05:09
@maribu
Copy link
Member Author

maribu commented Apr 25, 2025

Thx :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: doc Area: Documentation Area: pkg Area: External package ports Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants