From 30d2e592453969b6ab581a8b0c107bc45859cb16 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 24 Jun 2022 17:37:57 +0200 Subject: [PATCH 1/3] fix(core_debug): Make function non-static Static functions are private to the compilation unit they are emitted in, so they cannot be shared between compilation units. This means that any source file that uses `core_debug()` where the compiler does not inline (all) calls, will have its own private copy of this function emitted. In practice, gcc seems to never inline this function (even with -O3), leading to one copy of the function for each compilation unit it is used in. This fixes this by removing the `static` keyword from the function. However, this prevents the function from being emitted completely in C compilation units (C++ is different and emits multiple copies, discarding all but one later). This means that if the function is only used from C compilation units and not inlined everywhere, you get a linker error. Thet `static` keyword was probably added to work around this, without realizing the overhead. The proper way to prevent this linker error is to add an `extern` definition for the function in a single source file, so this adds a `core_debug.c` with exactly that. In practice, this means that this commit saves 40 bytes of space for each compilation unit where `core_debug()` is used (beyond the first). --- cores/arduino/core_debug.c | 5 +++++ cores/arduino/core_debug.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 cores/arduino/core_debug.c diff --git a/cores/arduino/core_debug.c b/cores/arduino/core_debug.c new file mode 100644 index 0000000000..d387df8342 --- /dev/null +++ b/cores/arduino/core_debug.c @@ -0,0 +1,5 @@ +#include "core_debug.h" + +// Ensure inline functions have a definition emitted for when they are +// not inlined (needed for C functions only) +extern void core_debug(const char *format, ...); diff --git a/cores/arduino/core_debug.h b/cores/arduino/core_debug.h index 98219db022..0c935cb515 100644 --- a/cores/arduino/core_debug.h +++ b/cores/arduino/core_debug.h @@ -16,7 +16,7 @@ extern "C" { * the code, use a lot of stack. An alternative, will be to implement a tiny * and limited functionality implementation of printf. */ -static inline void core_debug(const char *format, ...) +inline void core_debug(const char *format, ...) { #if !defined(NDEBUG) va_list args; From 337cad18be09f299bd9d00d46b9b2a75d6b2c91d Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 24 Jun 2022 17:49:23 +0200 Subject: [PATCH 2/3] feat: add vcore_debug function This does the same as core_debug, but (like printf vs vprintf) accepts an already processed va_list to allow other vararg functions to forward their argument lists to this function. --- cores/arduino/core_debug.c | 1 + cores/arduino/core_debug.h | 13 ++++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cores/arduino/core_debug.c b/cores/arduino/core_debug.c index d387df8342..8899b4c4d9 100644 --- a/cores/arduino/core_debug.c +++ b/cores/arduino/core_debug.c @@ -3,3 +3,4 @@ // Ensure inline functions have a definition emitted for when they are // not inlined (needed for C functions only) extern void core_debug(const char *format, ...); +extern void vcore_debug(const char *format, va_list args); diff --git a/cores/arduino/core_debug.h b/cores/arduino/core_debug.h index 0c935cb515..f5a74e62f3 100644 --- a/cores/arduino/core_debug.h +++ b/cores/arduino/core_debug.h @@ -1,8 +1,9 @@ #ifndef _CORE_DEBUG_H #define _CORE_DEBUG_H + +#include #if !defined(NDEBUG) #include - #include #endif /* NDEBUG */ #ifdef __cplusplus @@ -28,6 +29,16 @@ inline void core_debug(const char *format, ...) #endif /* NDEBUG */ } +inline void vcore_debug(const char *format, va_list args) +{ +#if !defined(NDEBUG) + vfprintf(stderr, format, args); +#else + (void)(format); + (void)(args); +#endif /* NDEBUG */ +} + #ifdef __cplusplus } #endif From b014a94fc09a64eda27752e5ed1375af8923c26b Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 24 Jun 2022 17:49:23 +0200 Subject: [PATCH 3/3] feat: add Print::vprintf function This does the same as printf, but (like the vprintf libc function) accepts an already processed va_list to allow other vararg functions to forward their argument lists to this function. --- cores/arduino/Print.cpp | 11 +++++++++++ cores/arduino/Print.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/cores/arduino/Print.cpp b/cores/arduino/Print.cpp index f0a0dc8f50..8b41f7c1ba 100644 --- a/cores/arduino/Print.cpp +++ b/cores/arduino/Print.cpp @@ -277,6 +277,17 @@ int Print::printf(const __FlashStringHelper *format, ...) return retval; } +int Print::vprintf(const char *format, va_list ap) +{ + return vdprintf((int)this, format, ap); +} + +int Print::vprintf(const __FlashStringHelper *format, va_list ap) +{ + return vdprintf((int)this, (const char *)format, ap); +} + + // Private Methods ///////////////////////////////////////////////////////////// size_t Print::printNumber(unsigned long n, uint8_t base) diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h index f7bae65254..036fd5347b 100644 --- a/cores/arduino/Print.h +++ b/cores/arduino/Print.h @@ -106,6 +106,8 @@ class Print { int printf(const char *format, ...); int printf(const __FlashStringHelper *format, ...); + int vprintf(const __FlashStringHelper *format, va_list ap); + int vprintf(const char *format, va_list ap); virtual void flush() { /* Empty implementation for backward compatibility */ } };