diff --git a/components/esp_stdio/Kconfig b/components/esp_stdio/Kconfig index a9b1da8c5e6..b89598510a6 100644 --- a/components/esp_stdio/Kconfig +++ b/components/esp_stdio/Kconfig @@ -185,23 +185,28 @@ menu "ESP-STDIO" If enabled, esp_rom_printf and ESP_EARLY_LOG output will also be sent over USB CDC. Disabling this option saves about 1kB or RAM. + config ESP_STDIO_BASIC_MODE + bool "Use basic /dev/console descriptor handling" + depends on VFS_SUPPORT_IO + default n + help + Enable the low-overhead legacy /dev/console descriptor handling. + + In basic mode, all open() calls share a single internal descriptor. This keeps RAM usage + lower, but disables checks for per-fd behaviour, such as read-only versus write-only access. + config ESP_STDIO_MAX_FDS int "Max logical stdio file descriptors" - depends on VFS_SUPPORT_IO - range 0 64 + depends on VFS_SUPPORT_IO && !ESP_STDIO_BASIC_MODE + range 3 64 default 16 help Maximum number of logical /dev/console descriptors tracked by esp_stdio VFS. - - 0 enables basic mode with a shared internal descriptor. - - 1..2 is not allowed - - Values >= 3 select descriptor-table mode (minimum table size is 3). stdin, stdout, and stderr each occupy one slot. The default leaves room for additional opens of /dev/console (e.g. dup2, reopen). If set to exactly 3, only the three standard streams fit and further opens fail with ENFILE unless one is closed first. - Enabling basic mode decreases RAM usage, but disables checks for per-fd behaviour, such as read-only. - config ESP_STDIO_MAX_VFS_ENTRIES int "Maximum number of registerable VFS sinks" depends on VFS_SUPPORT_IO diff --git a/components/esp_stdio/stdio_vfs.c b/components/esp_stdio/stdio_vfs.c index 2b2dab96140..dc5c6d9aad0 100644 --- a/components/esp_stdio/stdio_vfs.c +++ b/components/esp_stdio/stdio_vfs.c @@ -36,6 +36,7 @@ #include "esp_private/startup_internal.h" #include "esp_private/nullfs.h" #include "esp_heap_caps.h" +#include #endif #define STRINGIFY(s) STRINGIFY2(s) @@ -51,9 +52,11 @@ #if CONFIG_VFS_SUPPORT_IO -_Static_assert(CONFIG_ESP_STDIO_MAX_FDS == 0 || CONFIG_ESP_STDIO_MAX_FDS >= 3, "Invalid value for CONFIG_ESP_STDIO_MAX_FDS"); +#if !CONFIG_ESP_STDIO_BASIC_MODE +_Static_assert(CONFIG_ESP_STDIO_MAX_FDS >= 3, "Invalid value for CONFIG_ESP_STDIO_MAX_FDS"); +#endif -#define ESP_STDIO_IS_BASIC (CONFIG_ESP_STDIO_MAX_FDS <= 0) +#define ESP_STDIO_IS_BASIC (CONFIG_ESP_STDIO_BASIC_MODE) typedef struct { const esp_vfs_fs_ops_t *ops; @@ -80,10 +83,11 @@ typedef struct { } context_t; static context_t s_ctx = {0}; +static _lock_t s_ctx_lock; static const char *TAG = "esp_stdio"; -static inline bool is_fd_valid(int fd) +static inline bool is_fd_valid_nolock(int fd) { #if ESP_STDIO_IS_BASIC return (fd >= 0) && (s_ctx.fd_count > 0); @@ -92,6 +96,37 @@ static inline bool is_fd_valid(int fd) #endif } +static inline bool is_fd_valid(int fd) +{ + bool valid; + + _lock_acquire(&s_ctx_lock); + valid = is_fd_valid_nolock(fd); + _lock_release(&s_ctx_lock); + + return valid; +} + +#if !ESP_STDIO_IS_BASIC +static inline int get_fd_flags_nolock(int fd) +{ + assert((fd >= 0) && (fd < CONFIG_ESP_STDIO_MAX_FDS)); + return s_ctx.fds[fd].flags; +} + +static inline bool fd_allows_read_nolock(int fd) +{ + const int flags = get_fd_flags_nolock(fd); + return (flags & O_ACCMODE) != O_WRONLY; +} + +static inline bool fd_allows_write_nolock(int fd) +{ + const int flags = get_fd_flags_nolock(fd); + return (flags & O_ACCMODE) != O_RDONLY; +} +#endif + static inline mux_entry_t *get_primary_entry(void) { assert(s_ctx.used > 0); @@ -169,15 +204,26 @@ int console_open(__attribute__((unused)) void *ctx, const char * path, int flags { #if ESP_STDIO_IS_BASIC (void) path; + (void) flags; + (void) mode; + + _lock_acquire(&s_ctx_lock); s_ctx.fd_count++; + _lock_release(&s_ctx_lock); + return 0; #else + (void) mode; + if (!path || strcmp(path, "/") != 0) { errno = ENOENT; return -1; } + _lock_acquire(&s_ctx_lock); + if (s_ctx.fd_count >= CONFIG_ESP_STDIO_MAX_FDS) { + _lock_release(&s_ctx_lock); errno = ENFILE; return -1; } @@ -191,24 +237,28 @@ int console_open(__attribute__((unused)) void *ctx, const char * path, int flags } if (fd < 0) { + _lock_release(&s_ctx_lock); errno = ENFILE; return -1; } s_ctx.fd_count++; - s_ctx.fds[fd] = (fd_entry_t) { .in_use = true, .flags = flags, }; + _lock_release(&s_ctx_lock); return fd; #endif } int console_close(__attribute__((unused)) void *ctx, int fd) { - if (!is_fd_valid(fd)) { + _lock_acquire(&s_ctx_lock); + + if (!is_fd_valid_nolock(fd)) { + _lock_release(&s_ctx_lock); errno = EBADF; return -1; } @@ -220,23 +270,43 @@ int console_close(__attribute__((unused)) void *ctx, int fd) s_ctx.fd_count--; #endif + _lock_release(&s_ctx_lock); return 0; } ssize_t console_write(__attribute__((unused)) void *ctx, int fd, const void *data, size_t size) { +#if ESP_STDIO_IS_BASIC if (!is_fd_valid(fd)) { errno = EBADF; return -1; } - for (size_t i = 0; i < s_ctx.used; i++) { +#else + _lock_acquire(&s_ctx_lock); + if (!is_fd_valid_nolock(fd)) { + _lock_release(&s_ctx_lock); + errno = EBADF; + return -1; + } + if (!fd_allows_write_nolock(fd)) { + _lock_release(&s_ctx_lock); + errno = EBADF; + return -1; + } + _lock_release(&s_ctx_lock); +#endif + + const mux_entry_t *primary = get_primary_entry(); + ssize_t ret_val = primary->ops->write_p(primary->vfs_ctx, primary->fd, data, size); + + for (size_t i = 1; i < s_ctx.used; i++) { const mux_entry_t *entry = s_ctx.entries + i; if (entry->ops->write_p && entry->fd >= 0) { - entry->ops->write_p(entry->vfs_ctx, entry->fd, data, size); + (void) entry->ops->write_p(entry->vfs_ctx, entry->fd, data, size); } } - return size; + return ret_val; } int console_fstat(__attribute__((unused)) void *ctx, int fd, struct stat * st) @@ -256,10 +326,25 @@ int console_fstat(__attribute__((unused)) void *ctx, int fd, struct stat * st) ssize_t console_read(__attribute__((unused)) void *ctx, int fd, void * dst, size_t size) { +#if ESP_STDIO_IS_BASIC if (!is_fd_valid(fd)) { errno = EBADF; return -1; } +#else + _lock_acquire(&s_ctx_lock); + if (!is_fd_valid_nolock(fd)) { + _lock_release(&s_ctx_lock); + errno = EBADF; + return -1; + } + if (!fd_allows_read_nolock(fd)) { + _lock_release(&s_ctx_lock); + errno = EBADF; + return -1; + } + _lock_release(&s_ctx_lock); +#endif const mux_entry_t *entry = get_primary_entry(); if (!entry->ops->read_p) { @@ -302,7 +387,7 @@ int console_fsync(__attribute__((unused)) void *ctx, int fd) for (size_t i = 1; i < s_ctx.used; i++) { const mux_entry_t *entry = s_ctx.entries + i; if (entry->ops->fsync_p && entry->fd >= 0) { - entry->ops->fsync_p(entry->vfs_ctx, entry->fd); + (void) entry->ops->fsync_p(entry->vfs_ctx, entry->fd); } } @@ -325,13 +410,8 @@ int console_access(__attribute__((unused)) void *ctx, const char *path, int amod #ifdef CONFIG_VFS_SUPPORT_SELECT -/* - * Logical /dev/console fds (0,1,2,...) are not UART port numbers. uart_vfs select uses fd_set bits as - * SOC UART indices; without remapping, monitoring stdout/stderr selects UART1/UART2 and corrupts - * s_uart_select_count / ISR registration (only UART0 is the console sink). - */ typedef struct { - void *uart_args; + void *sink_select_args; fd_set *readfds; fd_set *writefds; fd_set *exceptfds; @@ -406,7 +486,7 @@ static esp_err_t console_start_select(int nfds, fd_set *readfds, fd_set *writefd return ESP_ERR_NO_MEM; } - ctx->uart_args = NULL; + ctx->sink_select_args = NULL; ctx->readfds = readfds; ctx->writefds = writefds; ctx->exceptfds = exceptfds; @@ -415,7 +495,7 @@ static esp_err_t console_start_select(int nfds, fd_set *readfds, fd_set *writefd ctx->interested_except = interested_except; esp_err_t err = entry->ops->select->start_select(forward_nfds, readfds, writefds, exceptfds, select_sem, - &ctx->uart_args); + &ctx->sink_select_args); if (err != ESP_OK) { if (readfds) { FD_ZERO(readfds); @@ -477,7 +557,7 @@ esp_err_t console_end_select(void *end_select_args) esp_err_t ret = ESP_OK; if (entry->fd >= 0 && entry->ops->select && entry->ops->select->end_select) { - ret = entry->ops->select->end_select(ctx->uart_args); + ret = entry->ops->select->end_select(ctx->sink_select_args); } const int hw = entry->fd; @@ -599,6 +679,7 @@ static const esp_vfs_fs_ops_t s_vfs_console = { esp_err_t esp_stdio_register(void) { esp_err_t err = ESP_OK; + _lock_init(&s_ctx_lock); // Primary vfs part. #if CONFIG_ESP_CONSOLE_UART const esp_vfs_fs_ops_t *ops = esp_vfs_uart_get_vfs(); diff --git a/components/esp_stdio/test_apps/stdio/main/test_app_main.c b/components/esp_stdio/test_apps/stdio/main/test_app_main.c index 154b83814b4..c6f5c8dffd3 100644 --- a/components/esp_stdio/test_apps/stdio/main/test_app_main.c +++ b/components/esp_stdio/test_apps/stdio/main/test_app_main.c @@ -52,7 +52,15 @@ static void console_none_print(void) #endif #if CONFIG_VFS_SUPPORT_IO -#if CONFIG_ESP_STDIO_MAX_FDS <= 0 || CONFIG_ESP_STDIO_MAX_FDS > 3 +#if CONFIG_ESP_STDIO_BASIC_MODE +#define ESP_STDIO_RUN_OPEN_CLOSE_CHECK 1 +#elif CONFIG_ESP_STDIO_MAX_FDS > 3 +#define ESP_STDIO_RUN_OPEN_CLOSE_CHECK 1 +#else +#define ESP_STDIO_RUN_OPEN_CLOSE_CHECK 0 +#endif + +#if ESP_STDIO_RUN_OPEN_CLOSE_CHECK static void console_open_close_check(void) { printf("Opening /dev/console\n"); @@ -72,7 +80,7 @@ static void console_open_close_check(void) static void stdio_fd_mode_behavior_check(void) { -#if CONFIG_ESP_STDIO_MAX_FDS <= 0 +#if CONFIG_ESP_STDIO_BASIC_MODE printf("STDIO_TEST:MODE=BASIC\n"); int fd0 = open("/dev/console", O_RDWR); @@ -87,6 +95,11 @@ static void stdio_fd_mode_behavior_check(void) ssize_t wr = write(fd0, msg, strlen(msg)); assert(wr == (ssize_t) strlen(msg)); + /* Verify write return value matches actual bytes (primary sink propagation) */ + const char *short_msg = "ab"; + wr = write(fd1, short_msg, 2); + assert(wr == 2); + assert(close(fd0) == 0); assert(close(fd1) == 0); assert(close(fd2) == 0); @@ -128,6 +141,35 @@ static void stdio_fd_mode_behavior_check(void) assert(close(fd2) == 0); assert(close(fd_reused) == 0); + int fd_ro = open("/dev/console", O_RDONLY); + assert(fd_ro >= 0); + errno = 0; + assert(write(fd_ro, "x", 1) < 0); + assert(errno == EBADF); + assert(close(fd_ro) == 0); + + int fd_wo = open("/dev/console", O_WRONLY); + assert(fd_wo >= 0); + errno = 0; + char tmp = 0; + assert(read(fd_wo, &tmp, 1) < 0); + assert(errno == EBADF); + assert(close(fd_wo) == 0); + + int fd_rw = open("/dev/console", O_RDWR); + assert(fd_rw >= 0); + const char *wmsg = "z"; + ssize_t wret = write(fd_rw, wmsg, 1); + assert(wret == 1); + assert(close(fd_rw) == 0); + printf("STDIO_TEST:NON_BASIC:FLAGS_OK\n"); + + int fd_fsync = open("/dev/console", O_RDWR); + assert(fd_fsync >= 0); + assert(fsync(fd_fsync) == 0); + assert(close(fd_fsync) == 0); + printf("STDIO_TEST:NON_BASIC:FSYNC_OK\n"); + int fds[CONFIG_ESP_STDIO_MAX_FDS + 2]; int opened = 0; while (opened < (int)(sizeof(fds) / sizeof(fds[0]))) { @@ -230,7 +272,7 @@ void app_main(void) #endif // CONFIG_ESP_CONSOLE_NONE #if CONFIG_VFS_SUPPORT_IO -#if CONFIG_ESP_STDIO_MAX_FDS <= 0 || CONFIG_ESP_STDIO_MAX_FDS > 3 +#if ESP_STDIO_RUN_OPEN_CLOSE_CHECK console_open_close_check(); #else printf("STDIO_TEST:SKIP:OPEN_CLOSE_CHECK\n"); diff --git a/components/esp_stdio/test_apps/stdio/pytest_esp_stdio_tests.py b/components/esp_stdio/test_apps/stdio/pytest_esp_stdio_tests.py index cd3ed8ef73e..61a7549a180 100644 --- a/components/esp_stdio/test_apps/stdio/pytest_esp_stdio_tests.py +++ b/components/esp_stdio/test_apps/stdio/pytest_esp_stdio_tests.py @@ -100,6 +100,8 @@ def test_esp_system_stdio_correct_open_and_close(dut: Dut) -> None: dut.expect('STDIO_TEST:MODE=NON_BASIC') dut.expect('STDIO_TEST:NON_BASIC:UNIQUE_OK') dut.expect('STDIO_TEST:NON_BASIC:REUSE_OK') + dut.expect('STDIO_TEST:NON_BASIC:FLAGS_OK') + dut.expect('STDIO_TEST:NON_BASIC:FSYNC_OK') dut.expect('STDIO_TEST:NON_BASIC:LIMIT_OK') dut.expect('STDIO_TEST:NON_BASIC:EBADF_OK') @@ -118,6 +120,8 @@ def test_esp_stdio_non_basic_default_qemu(dut: Dut) -> None: dut.expect('STDIO_TEST:MODE=NON_BASIC') dut.expect('STDIO_TEST:NON_BASIC:UNIQUE_OK') dut.expect('STDIO_TEST:NON_BASIC:REUSE_OK') + dut.expect('STDIO_TEST:NON_BASIC:FLAGS_OK') + dut.expect('STDIO_TEST:NON_BASIC:FSYNC_OK') dut.expect('STDIO_TEST:NON_BASIC:LIMIT_OK') dut.expect('STDIO_TEST:NON_BASIC:EBADF_OK') diff --git a/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_basic_mode b/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_basic_mode index 9f404e13d40..25d7a763ddc 100644 --- a/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_basic_mode +++ b/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_basic_mode @@ -1 +1 @@ -CONFIG_ESP_STDIO_MAX_FDS=0 +CONFIG_ESP_STDIO_BASIC_MODE=y diff --git a/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_non_basic_small_fd b/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_non_basic_small_fd index d077b890e82..3703a0c45a6 100644 --- a/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_non_basic_small_fd +++ b/components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_non_basic_small_fd @@ -1 +1,2 @@ +CONFIG_ESP_STDIO_BASIC_MODE=n CONFIG_ESP_STDIO_MAX_FDS=3