-
Notifications
You must be signed in to change notification settings - Fork 8.4k
libc/picolibc: Implement fdopen in terms of zvfs APIs #101449
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?
libc/picolibc: Implement fdopen in terms of zvfs APIs #101449
Conversation
58989e6 to
2ff8ff5
Compare
|
Two alternatives:
The first is 'more effiicient' as no-one will ever need a wrapper function. The second avoids adding POSIX APIs to the standard Zephyr suite. |
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| board_runner_args(openocd --cmd-reset-halt "reset halt") |
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.
doesn't seem related?
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.
oops, thanks -- this diff was hanging around after testing with a TI board...
| #ifdef CONFIG_PICOLIBC | ||
| return fileno(file); | ||
| #else |
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.
This adds a dependency cycle at the API level, so will not work.
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.
no, this is using fileno from picolibc, not Zephyr. Note that the Zephyr versions of fdopen and fileno should both be disabled by this patch.
| FUNC_ALIAS(close, _close, int); | ||
| #endif | ||
|
|
||
| #ifndef CONFIG_PICOLIBC |
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.
Can this PR be modified to implement POSIX_DEVICE_IO? Picolibc would need to add select TC_PROVIDES_POSIX_DEVICE_IO.
Ideally, this would only be merged after #97855, so that it could be done indpendently of the Zephyr implementation.
The Zephyr implementation should remain untouched, but lib/libc/picolibc/CMakeLists.txt could pull in Zephyr's implementation if it's convenient or simpler.
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.
All I'm trying to do in this PR is fix the current fdopen/fileno disaster. If you want to take it further, please feel free.
cfriedt
left a comment
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.
Please add select TC_PROVIDES_POSIX_DEVICE_IO under config PICOLIBC in lib/libc/Kconfig, implement (or borrow) the rest of POSIX_DEVICE_IO, and remove the layering violation in zvfs.
Use picolibc 'bufio' interfaces to provide a non-POSIX implementation of fdopen. This depends upon CONFIG_ZVFS_FDTABLE as that provides an fd abstraction on top of regular zephyr APIs. Use this API to provide zvfs_fdopen and zvfs_fileno. Signed-off-by: Keith Packard <[email protected]>
There is no layering violation; this removes the layering violation where zvfs provides broken versions of fdopen and fileno. |
2ff8ff5 to
a767405
Compare
|



Use picolibc 'bufio' interfaces to provide a non-POSIX implementation of fdopen. This depends upon CONFIG_ZVFS_FDTABLE as that provides an fd abstraction on top of regular zephyr APIs.
Use this API to provide zvfs_fdopen and zvfs_fileno.