zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.61k stars 6.5k forks source link

libc: implement `fread()` #66940

Open ycsin opened 9 months ago

ycsin commented 9 months ago

This was originally added as a ticket under POSIX, but fread() is not a POSIX function. It is from C89.

https://en.cppreference.com/w/c/io/fread

The expectation from the POSIX API is that this function is implemented as part of whatever C library is in use.

RAJAGOPALAN-GANGADHARAN commented 5 months ago

Hi @ycsin and @cfriedt , I would like to give this a shot. Can you please assign me this issue? I'm thinking of using this pr - https://github.com/zephyrproject-rtos/zephyr/pull/68516 as a base. Thanks!

cfriedt commented 5 months ago

@RAJAGOPALAN-GANGADHARAN - done!

However, something to keep in mind is that fread() is actually just a part of ISO C, all the way back to C89. That means, that the implementation of this function should likely go into the common libc, and tests should be added to the common libc test suite.

https://en.cppreference.com/w/c/io/fread

Additionally, a fairly fundamental problem needs to be solved for this to be done The Right Way (TM). Namely, we need a way to easily associate FILE * objects to (and from) integer file descriptors.

That would ideally be done in ZVFS (which is itself a kind of relocation of code from various libraries and subsystems to avoid dependency cycles at the API level.

RAJAGOPALAN-GANGADHARAN commented 5 months ago

Thanks @cfriedt - Looking at how fwrite has been implemented in minimal libc currently, seems like it only supports write to stdout and stderr. No fs support seems to be added (the use of zephyr fs apis like fs_open etc - https://docs.zephyrproject.org/latest/services/file_system/index.html#c.fs_dir_t.), is this what is expected from the fread (read from stdin) as well? Also in the future will the support be expanded to fs operations for these apis? Thanks!

cfriedt commented 5 months ago

@RAJAGOPALAN-GANGADHARAN Currently stdin / stdout / stderr should work, but we do want to support operations on other files as well.

de-nordic commented 5 months ago

@RAJAGOPALAN-GANGADHARAN - done!

However, something to keep in mind is that fread() is actually just a part of ISO C, all the way back to C89. That means, that the implementation of this function should likely go into the common libc, and tests should be added to the common libc test suite.

https://en.cppreference.com/w/c/io/fread

Additionally, a fairly fundamental problem needs to be solved for this to be done The Right Way (TM). Namely, we need a way to easily associate FILE * objects to (and from) integer file descriptors.

That would ideally be done in ZVFS (which is itself a kind of relocation of code from various libraries and subsystems to avoid dependency cycles at the API level.

@cfriedt Is correct.

C standard is subset of POSIX standard (or POSIX standard is superset over C). This can be seen when comparing read and fread functions using the OpenGroup documentation: read -> https://pubs.opengroup.org/onlinepubs/9699919799/functions/read.html fread -> https://pubs.opengroup.org/onlinepubs/9699919799/functions/fread.html

Look also at the C standard, draft, here https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf - you will notice that the fread is part of stdlib definition (Chapter 7).

Note also that C standard wins with POSIX when differs: image