-
Notifications
You must be signed in to change notification settings - Fork 8k
zvfs: improve libc FILE to integer fd abstraction (v3) #96278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e606b23
f156e34
1a32f98
dd819e4
6d6489c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,25 @@ | ||||||
/* | ||||||
* Copyright (c) 2025 Antmicro <www.antmicro.com> | ||||||
* | ||||||
* SPDX-License-Identifier: Apache-2.0 | ||||||
*/ | ||||||
|
||||||
#include <zephyr/sys/fdtable.h> | ||||||
#include <unistd.h> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
int fclose(FILE *stream) | ||||||
{ | ||||||
int fd; | ||||||
|
||||||
fflush(stream); | ||||||
fd = zvfs_libc_file_get_fd(stream); | ||||||
|
||||||
if (!fd) { | ||||||
return EOF; | ||||||
} | ||||||
if (close(fd)) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this would be more appropriate here, so that POSIX is not unintentionally pulled into the libc matters.
Suggested change
|
||||||
return EOF; | ||||||
} | ||||||
|
||||||
return 0; | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
/* | ||
* Copyright (c) 2024 Tenstorrent AI ULC | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
#include <stddef.h> | ||
#include <stdio.h> | ||
#include <stdlib.h> | ||
|
||
#include <zephyr/sys/fdtable.h> | ||
#include <zephyr/toolchain.h> | ||
|
||
BUILD_ASSERT(CONFIG_ZVFS_LIBC_FILE_SIZE >= sizeof(__FILE), | ||
"CONFIG_ZVFS_LIBC_FILE_SIZE must at least match size of FILE object"); | ||
BUILD_ASSERT(CONFIG_ZVFS_LIBC_FILE_ALIGN == __alignof(__FILE), | ||
"CONFIG_ZVFS_LIBC_FILE_SIZE must match alignment of FILE object"); | ||
|
||
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; | ||
} | ||
|
||
void zvfs_libc_file_alloc_cb(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; | ||
|
||
__ASSERT_NO_MSG(mode != NULL); | ||
__ASSERT_NO_MSG(fp != NULL); | ||
|
||
fp->_flags = z_libc_sflags(mode); | ||
fp->_file = fd; | ||
fp->_cookie = (void *)fp; | ||
|
||
*(uintptr_t *)&fp->_read = (uintptr_t)&_read_r; | ||
*(uintptr_t *)&fp->_write = (uintptr_t)&_write_r; | ||
*(uintptr_t *)&fp->_seek = (uintptr_t)&_lseek_r; | ||
*(uintptr_t *)&fp->_close = (uintptr_t)&_close_r; | ||
} | ||
|
||
int zvfs_libc_file_get_fd(FILE *fp) | ||
{ | ||
return fp->_file; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -200,4 +200,12 @@ config PICOLIBC_GLOBAL_ERRNO | |
|
||
endif # PICOLIBC_USE_MODULE | ||
|
||
if ZVFS | ||
|
||
config ZVFS_LIBC_FILE_SIZE | ||
default 144 if 64BIT | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Major: This seems a pretty tight assumption about the picolibc implementation (same for newlib) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also accurate. And also easily adjustable. |
||
default 80 | ||
|
||
endif # ZVFS | ||
|
||
endif # PICOLIBC |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,13 +5,20 @@ | |
*/ | ||
|
||
#include "picolibc-hooks.h" | ||
#include "stdio-bufio.h" | ||
|
||
static LIBC_DATA int (*_stdout_hook)(int); | ||
static int _stdout_hook_default(int c) | ||
{ | ||
(void)(c); /* Prevent warning about unused argument */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a macro for this |
||
|
||
return EOF; | ||
} | ||
|
||
static LIBC_DATA int (*_stdout_hook)(int) = _stdout_hook_default; | ||
|
||
int z_impl_zephyr_fputc(int a, FILE *out) | ||
{ | ||
(*_stdout_hook)(a); | ||
return 0; | ||
return ((out == stdout) || (out == stderr)) ? _stdout_hook(a) : EOF; | ||
} | ||
|
||
#ifdef CONFIG_USERSPACE | ||
|
@@ -24,31 +31,70 @@ static inline int z_vrfy_zephyr_fputc(int c, FILE *stream) | |
|
||
static int picolibc_put(char a, FILE *f) | ||
{ | ||
zephyr_fputc(a, f); | ||
return 0; | ||
return zephyr_fputc(a, f); | ||
} | ||
|
||
static LIBC_DATA FILE __stdout = FDEV_SETUP_STREAM(picolibc_put, NULL, NULL, 0); | ||
static LIBC_DATA FILE __stdin = FDEV_SETUP_STREAM(NULL, NULL, NULL, 0); | ||
#ifndef CONFIG_ZVFS | ||
static LIBC_DATA FILE __stdout = FDEV_SETUP_STREAM(picolibc_put, NULL, NULL, _FDEV_SETUP_WRITE); | ||
static LIBC_DATA FILE __stdin = FDEV_SETUP_STREAM(NULL, NULL, NULL, _FDEV_SETUP_READ); | ||
#endif | ||
|
||
#ifdef __strong_reference | ||
#define STDIO_ALIAS(x) __strong_reference(stdout, x); | ||
#else | ||
#define STDIO_ALIAS(x) FILE *const x = &__stdout; | ||
#endif | ||
|
||
#ifndef CONFIG_ZVFS | ||
FILE *const stdin = &__stdin; | ||
FILE *const stdout = &__stdout; | ||
STDIO_ALIAS(stderr); | ||
#endif | ||
|
||
void __stdout_hook_install(int (*hook)(int)) | ||
{ | ||
_stdout_hook = hook; | ||
__stdout.flags |= _FDEV_SETUP_WRITE; | ||
#ifdef CONFIG_ZVFS | ||
stdout->put = picolibc_put; | ||
stdout->flags |= _FDEV_SETUP_WRITE; | ||
|
||
struct __file_bufio *bp = (struct __file_bufio *)stdout; | ||
|
||
bp->ptr = INT_TO_POINTER(1 /* STDOUT_FILENO */); | ||
bp->bflags |= _FDEV_SETUP_WRITE; | ||
#endif | ||
} | ||
|
||
void __stdin_hook_install(unsigned char (*hook)(void)) | ||
{ | ||
__stdin.get = (int (*)(FILE *)) hook; | ||
__stdin.flags |= _FDEV_SETUP_READ; | ||
stdin->get = (int (*)(FILE *))hook; | ||
#ifdef CONFIG_ZVFS | ||
stdin->flags |= _FDEV_SETUP_READ; | ||
struct __file_bufio *bp = (struct __file_bufio *)stdin; | ||
|
||
bp->bflags |= _FDEV_SETUP_READ; | ||
bp->ptr = INT_TO_POINTER(0 /* STDIN_FILENO */); | ||
#endif | ||
} | ||
|
||
int z_impl_zephyr_write_stdout(const void *buffer, int nbytes) | ||
{ | ||
const char *buf = buffer; | ||
|
||
for (int i = 0; i < nbytes; i++) { | ||
if (*(buf + i) == '\n') { | ||
_stdout_hook('\r'); | ||
} | ||
_stdout_hook(*(buf + i)); | ||
} | ||
return nbytes; | ||
} | ||
|
||
#ifdef CONFIG_USERSPACE | ||
static inline int z_vrfy_zephyr_write_stdout(const void *buf, int nbytes) | ||
{ | ||
K_OOPS(K_SYSCALL_MEMORY_READ(buf, nbytes)); | ||
return z_impl_zephyr_write_stdout((const void *)buf, nbytes); | ||
} | ||
#include <zephyr/syscalls/zephyr_write_stdout_mrsh.c> | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In version 2 of this change set, it was required to avoid pulling in unnecessary dependencies which resulted in some issue on some platform.
In any case, it's a more accurate refinement of the dependency.