Skip to content

pkg/mpaland-printf: fix overriding printf with printf_float #21439

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 26, 2025

Conversation

maribu
Copy link
Member

@maribu maribu commented Apr 25, 2025

Contribution description

This makes sure that newlib's printf is correctly overridden by:

  1. Adding wrappers for some more obscure printf() variants
  2. Forcing a compilation failure when newlib's stdio is still linked in while mpaland-printf is used
  3. Not adding -u _printf_float to the linker flags when mpaland-printf is used.

Testing procedure

In master

$ make RIOT_CI_BUILD=1 BOARD=same54-xpro -C tests/core/thread_stack_alignment -j
make: Entering directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'
Building application "tests_thread_stack_alignment" for "same54-xpro" with CPU "samd5x".

   text	  data	   bss	   dec	   hex	filename
  28448	   492	  4612	 33552	  8310	/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment/bin/same54-xpro/tests_thread_stack_alignment.elf
make: Leaving directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'

$ USEPKG=mpaland-printf make RIOT_CI_BUILD=1 BOARD=same54-xpro -C tests/core/thread_stack_alignment -j
make: Entering directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'
Building application "tests_thread_stack_alignment" for "same54-xpro" with CPU "samd5x".

   text	  data	   bss	   dec	   hex	filename
  31648	   492	  4612	 36752	  8f90	/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment/bin/same54-xpro/tests_thread_stack_alignment.elf
make: Leaving directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'

In this PR

$ make RIOT_CI_BUILD=1 BOARD=same54-xpro -C tests/core/thread_stack_alignment -j
make: Entering directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'
Building application "tests_thread_stack_alignment" for "same54-xpro" with CPU "samd5x".

   text	  data	   bss	   dec	   hex	filename
  28448	   492	  4612	 33552	  8310	/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment/bin/same54-xpro/tests_thread_stack_alignment.elf
make: Leaving directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'

$ USEPKG=mpaland-printf make RIOT_CI_BUILD=1 BOARD=same54-xpro -C tests/core/thread_stack_alignment -j
make: Entering directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'
Building application "tests_thread_stack_alignment" for "same54-xpro" with CPU "samd5x".

   text	  data	   bss	   dec	   hex	filename
  17608	   128	  4608	 22344	  5748	/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment/bin/same54-xpro/tests_thread_stack_alignment.elf
make: Leaving directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/core/thread_stack_alignment'

Issues/PRs references

None

@github-actions github-actions bot added Area: pkg Area: External package ports Area: sys Area: System labels Apr 25, 2025
@crasbe crasbe added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 25, 2025
@riot-ci
Copy link

riot-ci commented Apr 25, 2025

Murdock results

✔️ PASSED

8e904a0 cpu/esp32: add workaround for mpaland-printf

Success Failures Total Runtime
10320 0 10320 12m:35s

Artifacts

@maribu maribu requested a review from gschorcht as a code owner April 25, 2025 12:36
@github-actions github-actions bot added Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Apr 25, 2025
Copy link
Contributor

@Enoch247 Enoch247 left a comment

Choose a reason for hiding this comment

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

A few nits. I'll let you decide if they are worth addressing or not. Also, I am wondering if some of the patches could be replaced with a glue module for the pkg to improve maintainability, but again, just a nit.

@Enoch247
Copy link
Contributor

Nice work! After a squash, this will be ready to merge.

maribu added 2 commits April 26, 2025 15:28
This makes sure that newlib's printf is correctly overridden by:

1. Adding wrappers for some more obscure printf() variants
2. Forcing a compilation failure when newlib's stdio is still linked in
   while mpaland-printf is used
3. Not adding `-u _printf_float` to the linker flags when mpaland-printf
   is used.
When mpaland-printf is used, we do not want to have references to
newlib's stdio in the resulting binary.
@maribu maribu force-pushed the pkg/mpaland-printf/fix-float branch from 083011b to 8e904a0 Compare April 26, 2025 13:28
@maribu maribu added this pull request to the merge queue Apr 26, 2025
Merged via the queue into RIOT-OS:master with commit fba75da Apr 26, 2025
25 checks passed
@maribu maribu deleted the pkg/mpaland-printf/fix-float branch April 27, 2025 06:24
@maribu
Copy link
Member Author

maribu commented Apr 27, 2025

Thx :-)

@Teufelchen1 Teufelchen1 added this to the Release 2025.07 milestone Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: pkg Area: External package ports Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants