From feab37096b16e5246c2ea61b27d99d4a3f71eac8 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Sun, 24 Feb 2019 13:17:31 -0800 Subject: [PATCH] libc: fix CONFIG_STDOUT_CONSOLE semantics The intent of this Kconfig is to allow libc stdout functions like printf() to send their output to the active console driver instead of discarding it. This somehow evolved into preferring to use printf() instead of printk() for all test case output if enabled. Libc printf() implementation for both minimal libc and newlib use considerably more stack space than printk(), with nothing gained by using them. Remove all instances where we are conditionally sending test case output based on this config, enable it by default, and adjust a few tests that disabled this because they were blowing stack. printk() and vprintk() now work as expected for unit_testing targets, they are just wrappers for host printf(). Fixes: #13701 Signed-off-by: Andrew Boie --- lib/libc/Kconfig | 2 +- .../crypto/mbedtls_benchmark/src/benchmark.c | 5 ---- samples/net/google_iot_mqtt/src/main.c | 7 ----- samples/net/google_iot_mqtt/src/protocol.c | 7 ----- samples/philosophers/src/main.c | 29 ++++-------------- .../cmsis_rtos_v1/philosophers/prj.conf | 1 - .../cmsis_rtos_v1/philosophers/src/main.c | 25 +++++----------- .../timer_synchronization/prj.conf | 1 - .../cmsis_rtos_v2/philosophers/prj.conf | 1 - .../cmsis_rtos_v2/philosophers/src/main.c | 30 +++++-------------- .../timer_synchronization/prj.conf | 1 - subsys/testsuite/include/tc_util.h | 7 +---- subsys/testsuite/ztest/include/ztest.h | 14 ++++----- subsys/testsuite/ztest/include/ztest_assert.h | 5 ---- subsys/testsuite/ztest/src/ztest_mock.c | 14 +++++++++ tests/crypto/mbedtls/src/mbedtls.c | 5 ---- tests/kernel/early_sleep/prj.conf | 2 -- tests/kernel/sleep/prj.conf | 4 +-- 18 files changed, 42 insertions(+), 118 deletions(-) diff --git a/lib/libc/Kconfig b/lib/libc/Kconfig index 8f9d153704..2d99256f94 100644 --- a/lib/libc/Kconfig +++ b/lib/libc/Kconfig @@ -48,7 +48,7 @@ config NEWLIB_LIBC_FLOAT_SCANF config STDOUT_CONSOLE bool "Send stdout to console" depends on CONSOLE_HAS_DRIVER - default NEWLIB_LIBC + default y help This option directs standard output (e.g. printf) to the console device, rather than suppressing it entirely. See also EARLY_CONSOLE diff --git a/samples/crypto/mbedtls_benchmark/src/benchmark.c b/samples/crypto/mbedtls_benchmark/src/benchmark.c index 75c5375d28..da589733a7 100644 --- a/samples/crypto/mbedtls_benchmark/src/benchmark.c +++ b/samples/crypto/mbedtls_benchmark/src/benchmark.c @@ -72,13 +72,8 @@ #include "kernel.h" -#if defined(CONFIG_STDOUT_CONSOLE) -#include -#define MBEDTLS_PRINT printf -#else #include #define MBEDTLS_PRINT ((int(*)(const char *, ...)) printk) -#endif /* CONFIG_STDOUT_CONSOLE */ static void my_debug(void *ctx, int level, const char *file, int line, const char *str) diff --git a/samples/net/google_iot_mqtt/src/main.c b/samples/net/google_iot_mqtt/src/main.c index 4dc6c95836..85d5dc98f6 100644 --- a/samples/net/google_iot_mqtt/src/main.c +++ b/samples/net/google_iot_mqtt/src/main.c @@ -23,13 +23,6 @@ LOG_MODULE_REGISTER(net_google_iot_mqtt, LOG_LEVEL_INF); #include #include -#ifdef CONFIG_STDOUT_CONSOLE -# include -# define PRINT printf -#else -# define PRINT printk -#endif - s64_t time_base; void do_sntp(struct addrinfo *addr) diff --git a/samples/net/google_iot_mqtt/src/protocol.c b/samples/net/google_iot_mqtt/src/protocol.c index 1c3511363a..2a3a9c131e 100644 --- a/samples/net/google_iot_mqtt/src/protocol.c +++ b/samples/net/google_iot_mqtt/src/protocol.c @@ -26,13 +26,6 @@ LOG_MODULE_DECLARE(net_google_iot_mqtt, LOG_LEVEL_DBG); #include -#ifdef CONFIG_STDOUT_CONSOLE -# include -# define PRINT printf -#else -# define PRINT printk -#endif - extern s64_t time_base; /* private key information */ diff --git a/samples/philosophers/src/main.c b/samples/philosophers/src/main.c index bd3e03d01e..87576883df 100644 --- a/samples/philosophers/src/main.c +++ b/samples/philosophers/src/main.c @@ -85,23 +85,6 @@ #define STACK_SIZE 768 -/* - * There are multiple threads doing printfs and they may conflict. - * Therefore use puts() instead of printf(). - */ -#if defined(CONFIG_STDOUT_CONSOLE) -#define PRINTF(...) { char output[256]; \ - sprintf(output, __VA_ARGS__); puts(output); } -#else -#define PRINTF(...) printk(__VA_ARGS__) -#endif - -#if DEBUG_PRINTF -#define PR_DEBUG PRINTF -#else -#define PR_DEBUG(...) -#endif - #include "phil_obj_abstract.h" #define fork(x) (forks[x]) @@ -109,7 +92,7 @@ static void set_phil_state_pos(int id) { #if !DEBUG_PRINTF - PRINTF("\x1b[%d;%dH", id + 1, 1); + printk("\x1b[%d;%dH", id + 1, 1); #endif } @@ -120,18 +103,18 @@ static void print_phil_state(int id, const char *fmt, s32_t delay) set_phil_state_pos(id); - PRINTF("Philosopher %d [%s:%s%d] ", + printk("Philosopher %d [%s:%s%d] ", id, prio < 0 ? "C" : "P", prio < 0 ? "" : " ", prio); if (delay) { - PRINTF(fmt, delay < 1000 ? " " : "", delay); + printk(fmt, delay < 1000 ? " " : "", delay); } else { - PRINTF(fmt, ""); + printk(fmt, ""); } - PRINTF("\n"); + printk("\n"); } static s32_t get_random_delay(int id, int period_in_ms) @@ -257,7 +240,7 @@ static void start_threads(void) static void display_demo_description(void) { #if !DEBUG_PRINTF - PRINTF(DEMO_DESCRIPTION); + printk(DEMO_DESCRIPTION); #endif } diff --git a/samples/portability/cmsis_rtos_v1/philosophers/prj.conf b/samples/portability/cmsis_rtos_v1/philosophers/prj.conf index 4a07a5eca3..7c657b272d 100644 --- a/samples/portability/cmsis_rtos_v1/philosophers/prj.conf +++ b/samples/portability/cmsis_rtos_v1/philosophers/prj.conf @@ -1,4 +1,3 @@ -CONFIG_STDOUT_CONSOLE=n CONFIG_SYS_CLOCK_TICKS_PER_SEC=100 CONFIG_ASSERT=y CONFIG_ASSERT_LEVEL=2 diff --git a/samples/portability/cmsis_rtos_v1/philosophers/src/main.c b/samples/portability/cmsis_rtos_v1/philosophers/src/main.c index 0c9bdd9d22..486fc35644 100644 --- a/samples/portability/cmsis_rtos_v1/philosophers/src/main.c +++ b/samples/portability/cmsis_rtos_v1/philosophers/src/main.c @@ -74,19 +74,8 @@ osSemaphoreId forks[NUM_PHIL]; #define STACK_SIZE 512 -/* - * There are multiple threads doing printfs and they may conflict. - * Therefore use puts() instead of printf(). - */ -#if defined(CONFIG_STDOUT_CONSOLE) -#define PRINTF(...) { char output[256]; \ - sprintf(output, __VA_ARGS__); puts(output); } -#else -#define PRINTF(...) printk(__VA_ARGS__) -#endif - #if DEBUG_PRINTF -#define PR_DEBUG PRINTF +#define PR_DEBUG printk #else #define PR_DEBUG(...) #endif @@ -96,7 +85,7 @@ osSemaphoreId forks[NUM_PHIL]; static void set_phil_state_pos(int id) { #if !DEBUG_PRINTF - PRINTF("\x1b[%d;%dH", id + 1, 1); + printk("\x1b[%d;%dH", id + 1, 1); #endif } @@ -107,18 +96,18 @@ static void print_phil_state(int id, const char *fmt, s32_t delay) set_phil_state_pos(id); - PRINTF("Philosopher %d [%s:%s%d] ", + printk("Philosopher %d [%s:%s%d] ", id, prio < 0 ? "C" : "P", prio < 0 ? "" : " ", prio); if (delay) { - PRINTF(fmt, delay < 1000 ? " " : "", delay); + printk(fmt, delay < 1000 ? " " : "", delay); } else { - PRINTF(fmt, ""); + printk(fmt, ""); } - PRINTF("\n"); + printk("\n"); } static s32_t get_random_delay(int id, int period_in_ms) @@ -215,7 +204,7 @@ static void start_threads(void) static void display_demo_description(void) { #if !DEBUG_PRINTF - PRINTF(DEMO_DESCRIPTION); + printk(DEMO_DESCRIPTION); #endif } diff --git a/samples/portability/cmsis_rtos_v1/timer_synchronization/prj.conf b/samples/portability/cmsis_rtos_v1/timer_synchronization/prj.conf index 4a07a5eca3..7c657b272d 100644 --- a/samples/portability/cmsis_rtos_v1/timer_synchronization/prj.conf +++ b/samples/portability/cmsis_rtos_v1/timer_synchronization/prj.conf @@ -1,4 +1,3 @@ -CONFIG_STDOUT_CONSOLE=n CONFIG_SYS_CLOCK_TICKS_PER_SEC=100 CONFIG_ASSERT=y CONFIG_ASSERT_LEVEL=2 diff --git a/samples/portability/cmsis_rtos_v2/philosophers/prj.conf b/samples/portability/cmsis_rtos_v2/philosophers/prj.conf index 89753bb53d..57a6fcb2bd 100644 --- a/samples/portability/cmsis_rtos_v2/philosophers/prj.conf +++ b/samples/portability/cmsis_rtos_v2/philosophers/prj.conf @@ -1,4 +1,3 @@ -CONFIG_STDOUT_CONSOLE=n CONFIG_SYS_CLOCK_TICKS_PER_SEC=100 CONFIG_ASSERT=y CONFIG_ASSERT_LEVEL=2 diff --git a/samples/portability/cmsis_rtos_v2/philosophers/src/main.c b/samples/portability/cmsis_rtos_v2/philosophers/src/main.c index 6c5e5c0cda..353d1e030f 100644 --- a/samples/portability/cmsis_rtos_v2/philosophers/src/main.c +++ b/samples/portability/cmsis_rtos_v2/philosophers/src/main.c @@ -31,12 +31,7 @@ #include #include #include - -#if defined(CONFIG_STDOUT_CONSOLE) -#include -#else #include -#endif #include @@ -115,19 +110,8 @@ static osThreadAttr_t thread_attr[] = { }; -/* - * There are multiple threads doing printfs and they may conflict. - * Therefore use puts() instead of printf(). - */ -#if defined(CONFIG_STDOUT_CONSOLE) -#define PRINTF(...) { char output[256]; \ - sprintf(output, __VA_ARGS__); puts(output); } -#else -#define PRINTF(...) printk(__VA_ARGS__) -#endif - #if DEBUG_PRINTF -#define PR_DEBUG PRINTF +#define PR_DEBUG printk #else #define PR_DEBUG(...) #endif @@ -137,7 +121,7 @@ static osThreadAttr_t thread_attr[] = { static void set_phil_state_pos(int id) { #if !DEBUG_PRINTF - PRINTF("\x1b[%d;%dH", id + 1, 1); + printk("\x1b[%d;%dH", id + 1, 1); #endif } @@ -148,18 +132,18 @@ static void print_phil_state(int id, const char *fmt, s32_t delay) set_phil_state_pos(id); - PRINTF("Philosopher %d [%s:%s%d] ", + printk("Philosopher %d [%s:%s%d] ", id, prio < 0 ? "C" : "P", prio < 0 ? "" : " ", prio); if (delay) { - PRINTF(fmt, delay < 1000 ? " " : "", delay); + printk(fmt, delay < 1000 ? " " : "", delay); } else { - PRINTF(fmt, ""); + printk(fmt, ""); } - PRINTF("\n"); + printk("\n"); } static s32_t get_random_delay(int id, int period_in_ms) @@ -259,7 +243,7 @@ static void start_threads(void) static void display_demo_description(void) { #if !DEBUG_PRINTF - PRINTF(DEMO_DESCRIPTION); + printk(DEMO_DESCRIPTION); #endif } diff --git a/samples/portability/cmsis_rtos_v2/timer_synchronization/prj.conf b/samples/portability/cmsis_rtos_v2/timer_synchronization/prj.conf index 89753bb53d..57a6fcb2bd 100644 --- a/samples/portability/cmsis_rtos_v2/timer_synchronization/prj.conf +++ b/samples/portability/cmsis_rtos_v2/timer_synchronization/prj.conf @@ -1,4 +1,3 @@ -CONFIG_STDOUT_CONSOLE=n CONFIG_SYS_CLOCK_TICKS_PER_SEC=100 CONFIG_ASSERT=y CONFIG_ASSERT_LEVEL=2 diff --git a/subsys/testsuite/include/tc_util.h b/subsys/testsuite/include/tc_util.h index 07b244ee57..8fcf0a6522 100644 --- a/subsys/testsuite/include/tc_util.h +++ b/subsys/testsuite/include/tc_util.h @@ -13,14 +13,9 @@ #include #include - -#if defined(CONFIG_STDOUT_CONSOLE) -#include -#define PRINT_DATA(fmt, ...) printf(fmt, ##__VA_ARGS__) -#else #include + #define PRINT_DATA(fmt, ...) printk(fmt, ##__VA_ARGS__) -#endif /* CONFIG_STDOUT_CONSOLE */ #if defined CONFIG_ARCH_POSIX #include "posix_board_if.h" diff --git a/subsys/testsuite/ztest/include/ztest.h b/subsys/testsuite/ztest/include/ztest.h index 4d7f897dbe..4d0dacaa17 100644 --- a/subsys/testsuite/ztest/include/ztest.h +++ b/subsys/testsuite/ztest/include/ztest.h @@ -43,18 +43,14 @@ extern "C" { #define CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC 10000000 /* FIXME: Properly integrate with Zephyr's arch specific code */ #define CONFIG_X86 1 -#define PRINT printf -#endif /* !KERNEL */ +#define CONFIG_PRINTK 1 +#endif + +#include +#define PRINT printk #include -#if defined(CONFIG_STDOUT_CONSOLE) -#include -#define PRINT printf -#else /* !CONFIG_STDOUT_CONSOLE */ -#include -#define PRINT printk -#endif /* CONFIG_STDOUT_CONSOLE */ #include #include diff --git a/subsys/testsuite/ztest/include/ztest_assert.h b/subsys/testsuite/ztest/include/ztest_assert.h index 4c7ecaac92..4666339f32 100644 --- a/subsys/testsuite/ztest/include/ztest_assert.h +++ b/subsys/testsuite/ztest/include/ztest_assert.h @@ -48,13 +48,8 @@ static inline void _zassert(int cond, va_start(vargs, msg); PRINT("\n Assertion failed at %s:%d: %s: %s\n", file, line, func, default_msg); -#if defined(CONFIG_STDOUT_CONSOLE) - vprintf(msg, vargs); - printf("\n"); -#else vprintk(msg, vargs); printk("\n"); -#endif va_end(vargs); ztest_test_fail(); } diff --git a/subsys/testsuite/ztest/src/ztest_mock.c b/subsys/testsuite/ztest/src/ztest_mock.c index b87d37f886..707ecd3a43 100644 --- a/subsys/testsuite/ztest/src/ztest_mock.c +++ b/subsys/testsuite/ztest/src/ztest_mock.c @@ -19,6 +19,7 @@ struct parameter { #ifndef KERNEL #include +#include static void free_parameter(struct parameter *param) { @@ -43,6 +44,19 @@ void _init_mock(void) } +void printk(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vprintf(fmt, ap); + va_end(ap); +} + +void vprintk(const char *fmt, va_list ap) +{ + vprintf(fmt, ap); +} #else /* diff --git a/tests/crypto/mbedtls/src/mbedtls.c b/tests/crypto/mbedtls/src/mbedtls.c index 7028e55df4..92ed80b5ad 100644 --- a/tests/crypto/mbedtls/src/mbedtls.c +++ b/tests/crypto/mbedtls/src/mbedtls.c @@ -7,13 +7,8 @@ * This file is part of mbed TLS (https://tls.mbed.org) */ -#if defined(CONFIG_STDOUT_CONSOLE) -#include -#define MBEDTLS_PRINT printf -#else #include #define MBEDTLS_PRINT (int(*)(const char *, ...)) printk -#endif /* CONFIG_STDOUT_CONSOLE */ #include #include diff --git a/tests/kernel/early_sleep/prj.conf b/tests/kernel/early_sleep/prj.conf index 141f07ecd0..850460f9df 100644 --- a/tests/kernel/early_sleep/prj.conf +++ b/tests/kernel/early_sleep/prj.conf @@ -1,4 +1,2 @@ CONFIG_IRQ_OFFLOAD=y CONFIG_ZTEST=y -# May fail without this -CONFIG_STDOUT_CONSOLE=n diff --git a/tests/kernel/sleep/prj.conf b/tests/kernel/sleep/prj.conf index 4a1d7c0ab6..cee5de8e83 100644 --- a/tests/kernel/sleep/prj.conf +++ b/tests/kernel/sleep/prj.conf @@ -1,5 +1,3 @@ CONFIG_IRQ_OFFLOAD=y CONFIG_ZTEST=y -# May fail without this -CONFIG_STDOUT_CONSOLE=n -CONFIG_QEMU_TICKLESS_WORKAROUND=y \ No newline at end of file +CONFIG_QEMU_TICKLESS_WORKAROUND=y