From e2dd67e1035ca420248d1fb66206366d79a9f92d Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Wed, 25 Dec 2024 17:01:39 -0500 Subject: [PATCH 1/2] libc: newlib: hooks: correct signature for open() The function signature for open() in libc-hooks.c was incorrect. Additionally, the ALIAS_OF() macro causes a compile error, because it simplifies the open function signature to int open(), which is a conflicting definition. Use the correct definition to get around compile errors. Signed-off-by: Chris Friedt --- lib/libc/newlib/libc-hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libc/newlib/libc-hooks.c b/lib/libc/newlib/libc-hooks.c index ba0c8efcdd96..4bf7bb251e05 100644 --- a/lib/libc/newlib/libc-hooks.c +++ b/lib/libc/newlib/libc-hooks.c @@ -243,7 +243,7 @@ int _open(const char *name, int flags, ...) { return -1; } -__weak FUNC_ALIAS(_open, open, int); +__weak int open(const char *name, int mode, ...) ALIAS_OF(_open); int _close(int file) { From 98991eb750bd54d139bc23cdcfc6c284eb195c2e Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Tue, 24 Dec 2024 12:44:46 -0500 Subject: [PATCH 2/2] zvfs: improve libc FILE to integer fd abstraction Previously, there was an implicit assumption that Zephyr's internal struct fd_entry * was synonymous with FILE * from the C library. This is generally not the case and aliasing these two distinct types was preventing a fair bit of functionality from Just Working - namely stdio function calls like fgets() and fopen(). The problem count be seen directly when trying to use a function like zvfs_fdopen(). Instead of aliasing the two types, require that all Zephyr C libraries provide 1. FILE *z_libc_file_alloc(int fd, const char *mode) Allocate and populate the required fields of a FILE object 2. int z_libc_file_get_fd(FILE *fp) Convert a FILE* object to an integer file descriptor. For Picolibc and Newlib-based C libraries, these functions set and get the integer file descriptor from a field of the internal FILE object representation. For the minimal C library, these functions convert between array index and struct fd_entry pointers. Signed-off-by: Chris Friedt --- lib/libc/newlib/CMakeLists.txt | 5 +- lib/libc/newlib/z_libc.c | 81 ++++++++++++++++++++++++++++++++ lib/libc/picolibc/CMakeLists.txt | 1 + lib/libc/picolibc/z_libc.c | 60 +++++++++++++++++++++++ lib/os/fdtable.c | 34 ++++++++++++-- 5 files changed, 176 insertions(+), 5 deletions(-) create mode 100644 lib/libc/newlib/z_libc.c create mode 100644 lib/libc/picolibc/z_libc.c diff --git a/lib/libc/newlib/CMakeLists.txt b/lib/libc/newlib/CMakeLists.txt index d3dc448eccaf..0e9e7fbcb0e4 100644 --- a/lib/libc/newlib/CMakeLists.txt +++ b/lib/libc/newlib/CMakeLists.txt @@ -1,7 +1,10 @@ # SPDX-License-Identifier: Apache-2.0 zephyr_library() -zephyr_library_sources(libc-hooks.c) +zephyr_library_sources( + libc-hooks.c + z_libc.c +) # Do not allow LTO when compiling libc-hooks.c file set_source_files_properties(libc-hooks.c PROPERTIES COMPILE_OPTIONS $) diff --git a/lib/libc/newlib/z_libc.c b/lib/libc/newlib/z_libc.c new file mode 100644 index 000000000000..2faa31829779 --- /dev/null +++ b/lib/libc/newlib/z_libc.c @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2024 Tenstorrent AI ULC + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include + +#include + +static int z_libc_sflags(const char *mode) +{ + int ret = 0; + + switch (mode[0]) { + case 'r': + ret = ZVFS_O_RDONLY; + break; + + case 'w': + ret = ZVFS_O_WRONLY | ZVFS_O_CREAT | ZVFS_O_TRUNC; + break; + + case 'a': + ret = ZVFS_O_WRONLY | ZVFS_O_CREAT | ZVFS_O_APPEND; + break; + default: + return 0; + } + + while (*++mode) { + switch (*mode) { + case '+': + ret |= ZVFS_O_RDWR; + break; + case 'x': + ret |= ZVFS_O_EXCL; + break; + default: + break; + } + } + + return ret; +} + +FILE *z_libc_file_alloc(int fd, const char *mode) +{ + FILE *fp; + /* + * These symbols have conflicting declarations in upstream headers. + * Use uintptr_t and a cast to assign cleanly. + */ + extern uintptr_t _close_r; + extern uintptr_t _lseek_r; + extern uintptr_t _read_r; + extern uintptr_t _write_r; + + fp = calloc(1, sizeof(*fp)); + if (fp == NULL) { + return NULL; + } + + fp->_flags = z_libc_sflags(mode); + fp->_file = fd; + fp->_cookie = (void *)fp; + + *(uintptr_t *)fp->_read = _read_r; + *(uintptr_t *)fp->_write = _write_r; + *(uintptr_t *)fp->_seek = _lseek_r; + *(uintptr_t *)fp->_close = _close_r; + + return fp; +} + +int z_libc_file_get_fd(FILE *fp) +{ + return fp->_file; +} diff --git a/lib/libc/picolibc/CMakeLists.txt b/lib/libc/picolibc/CMakeLists.txt index 752379dd76d4..ff83af9b8dbd 100644 --- a/lib/libc/picolibc/CMakeLists.txt +++ b/lib/libc/picolibc/CMakeLists.txt @@ -9,6 +9,7 @@ zephyr_library_sources( exit.c locks.c stdio.c + z_libc.c ) zephyr_library_compile_options($) diff --git a/lib/libc/picolibc/z_libc.c b/lib/libc/picolibc/z_libc.c new file mode 100644 index 000000000000..79069238a307 --- /dev/null +++ b/lib/libc/picolibc/z_libc.c @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2024 Tenstorrent AI ULC + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "stdio-bufio.h" + +#include +#include +#include + +#include + +#define FDEV_SETUP_ZVFS(fd, buf, size, rwflags, bflags) \ + FDEV_SETUP_BUFIO(fd, buf, size, zvfs_read_wrap, zvfs_write_wrap, zvfs_lseek, zvfs_close, \ + rwflags, bflags) + +int __posix_sflags(const char *mode, int *optr); + +/* FIXME: do not use ssize_t or off_t */ +ssize_t zvfs_read(int fd, void *buf, size_t sz, const size_t *from_offset); +ssize_t zvfs_write(int fd, const void *buf, size_t sz, const size_t *from_offset); +off_t zvfs_lseek(int fd, off_t offset, int whence); +int zvfs_close(int fd); + +static ssize_t zvfs_read_wrap(int fd, void *buf, size_t sz) +{ + return zvfs_read(fd, buf, sz, NULL); +} + +static ssize_t zvfs_write_wrap(int fd, const void *buf, size_t sz) +{ + return zvfs_write(fd, buf, sz, NULL); +} + +FILE *z_libc_file_alloc(int fd, const char *mode) +{ + struct __file_bufio *bf; + int __maybe_unused unused; + + bf = calloc(1, sizeof(struct __file_bufio) + BUFSIZ); + if (bf == NULL) { + return NULL; + } + + *bf = (struct __file_bufio)FDEV_SETUP_ZVFS(fd, (char *)(bf + 1), BUFSIZ, + __posix_sflags(mode, &unused), __BFALL); + + __bufio_lock_init(&(bf->xfile.cfile.file)); + + return &(bf->xfile.cfile.file); +} + +int z_libc_file_get_fd(const FILE *fp) +{ + const struct __file_bufio *const bf = (const struct __file_bufio *)fp; + + return POINTER_TO_INT(bf->ptr); +} diff --git a/lib/os/fdtable.c b/lib/os/fdtable.c index 0299e89812a3..3ed09c042623 100644 --- a/lib/os/fdtable.c +++ b/lib/os/fdtable.c @@ -24,6 +24,11 @@ #include #include +#ifndef CONFIG_MINIMAL_LIBC +extern FILE *z_libc_file_alloc(int fd, const char *mode); +extern int z_libc_file_get_fd(const FILE *fp); +#endif + struct stat; struct fd_entry { @@ -75,6 +80,20 @@ static struct fd_entry fdtable[CONFIG_ZVFS_OPEN_MAX] = { static K_MUTEX_DEFINE(fdtable_lock); +#ifdef CONFIG_MINIMAL_LIBC +static ALWAYS_INLINE inline FILE *z_libc_file_alloc(int fd, const char *mode) +{ + ARG_UNUSED(mode); + + return (FILE *)&fdtable[fd]; +} + +static ALWAYS_INLINE inline int z_libc_file_get_fd(const FILE *fp) +{ + return (const struct fd_entry *)fp - fdtable; +} +#endif + static int z_fd_ref(int fd) { return atomic_inc(&fdtable[fd].refcount) + 1; @@ -413,23 +432,30 @@ int zvfs_close(int fd) FILE *zvfs_fdopen(int fd, const char *mode) { - ARG_UNUSED(mode); + FILE *ret; if (_check_fd(fd) < 0) { return NULL; } - return (FILE *)&fdtable[fd]; + ret = z_libc_file_alloc(fd, mode); + if (ret == NULL) { + errno = ENOMEM; + } + + return ret; } int zvfs_fileno(FILE *file) { - if (!IS_ARRAY_ELEMENT(fdtable, file)) { + int fd = z_libc_file_get_fd(file); + + if (fd < 0 || fd >= ARRAY_SIZE(fdtable)) { errno = EBADF; return -1; } - return (struct fd_entry *)file - fdtable; + return fd; } int zvfs_fstat(int fd, struct stat *buf)