core: add stdio.h to replace stdout functions with stdio_null#20872
core: add stdio.h to replace stdout functions with stdio_null#20872benpicco merged 5 commits intoRIOT-OS:masterfrom
Conversation
maribu
left a comment
There was a problem hiding this comment.
lgtm. The CI has complaints, though
9f4137e to
55aedc0
Compare
|
|
||
| #ifndef CORE_STDIO_H | ||
| #define CORE_STDIO_H | ||
| #include_next "stdio.h" |
There was a problem hiding this comment.
That's a GNU extension, AIU supported by Clang as well.
I'm not saying we shouldn't do this (I'm glad of all libc headers we can do ourselves to avoid weird choices by the used libc), but it is in direct conflict with the first bulled of the coding convention. But if we start doing it, better change the code convention (or if that's what we do get a 2-ACK dispensation), and reap the fruit by using #pragma once, rather than creating a divide between what we do and what we claim to do.
There was a problem hiding this comment.
+1 for updating the coding convention to match.
We actually already have an used of #include_next in the AVR code to automatically move format string to flash and use printf_P() to save lots of RAM.
There was a problem hiding this comment.
I added a line to CODING_CONVENTIONS.md.
I think being implemented by both GCC and Clang is enough of a safeguard to prevent opening the gates to some crazy things.
There was a problem hiding this comment.
We already use #include_next in esp32, tinyusb, posix and avr8_common - is the stdio case really any different?
There was a problem hiding this comment.
I'd be more lenient in the cpu directories where there might easily just not be compilers other than the CPU specific one (AIU, neither ESP32 nor AVR can be built with off-the-shelf C compilers). Similarly in the POSIX case, its use is limited to native, which AIU also comes with a severe set of limitations by construction.
As for tinyusb and POSIX, I'd guess that neither reviewer nor author took care.
There was a problem hiding this comment.
AVR can use upstream GCC (and we do so). Upstream clang is rather mature, but not feature complete.
ESP does indeed need magic binary only compilers.
1f6624b to
1280166
Compare
1280166 to
3cd06eb
Compare
|
How do we proceed here? |
|
With the feature freeze past, #21140 in and one approval present, I think we can go on and merge. |
|
Ping 😉 |
cc80e34 to
183ad72
Compare
183ad72 to
425fb5b
Compare
We can't use the same header guard if we include two files with the same name.
425fb5b to
f840d80
Compare
Contribution description
So far
stdio_nullwould just drop the stdio backend, all the expensive stdio formatting functions were still included.This PR replaces all calls to
printf()and friends with a no-op.Testing procedure
Build e.g.
tests/minimalwhich usesstdio_null:master
this PR
Issues/PRs references