posix: Clean up of struct timeval define and related functions #15628
Conversation
|
All checks are passing now. Review history of this comment for details about previous failed status. |
pfalcon
force-pushed
the
posix-time-defs
branch
from
2b27fad to
9306020
Compare
|
This now passed CI. I had to apply an ugly workaround for Xtensa due to zephyrproject-rtos/sdk-ng#64 . (Note that my preference would be blacklist Xtensa or any other non-compliant archs, until they're brought in line with other archs.) |
| /* Kludge to support outdated newlib version as used in SDK 0.10 for Xtensa */ | ||
| #include <newlib.h> | ||
|
|
||
| #ifdef __NEWLIB__ |
There was a problem hiding this comment.
Can you add some more comments here about what's going on. I'm guessing this is all because of xtensa's version of newlib being very old.
There was a problem hiding this comment.
Comment 2 lines above of "Kludge to support outdated newlib version as used in SDK 0.10 for Xtensa" not enough?
| #include <newlib.h> | ||
|
|
||
| #ifdef __NEWLIB__ | ||
| #include <sys/_timeval.h> |
There was a problem hiding this comment.
Are you expecting more things to end up in _timeval.h? Just curious why to introduce a new file instead of just putting the struct here directly.
There was a problem hiding this comment.
We discussed that at Connect and commit message explains - I'm introduce that file because newlib has that file with that define. It gets included by various other newlib headers. If we have concurrent definition in some other file (like Zephyr POSIX header), we get conflict. We should not play against newlib, but along with it, reusing its definitions. But then the other libc (minimal) should follow the same file structuring and naming, or we'll never get all the different configurations to work consistently.
Now why newlib put that declaration in a separate is a different matter. I can theorize that as mentioned, struct timeval declaration is needed in various headers, e.g. select.h. But nowhere POSIX would say something like "including select.h should have the effect of including sys/time.h". That's why struct timeval is factored out to a separate (libc-private) header, and fine-grained inclusion happens.
According to POSIX, these types should be defined by sys/types.h. Signed-off-by: Paul Sokolovsky <[email protected]>
This is implementation-level header which defines struct timeval, and intended to be included by headers which need this structure. This implementation scheme is compatible with Newlib, and thus provides a step to use minlibc vs Newlib interchangeably. Signed-off-by: Paul Sokolovsky <[email protected]>
According to POSIX, that's the header which defines this function. Similarly, nothing in POSIX indicates that <time.h> should have access to struct timeval, so it's removed (it's made accessible to <sys/time.h> via <sys/_timeval.h> introduced earlier). Signed-off-by: Paul Sokolovsky <[email protected]>
Unfortunately, Zephyr SDK 0.10.0 ships with outdate Newlib 2.0.0 (from 2015 or earlier) which lacks sys/_timeval.h header, requiring ugly workaround of defining struct timeval inline (the whole idea was to standardize on sys/_timeval.h header for different libc's). Signed-off-by: Paul Sokolovsky <[email protected]>
Add fallback maintainer for lib/libc. Signed-off-by: Paul Sokolovsky <[email protected]>
|
@PiotrZierhoffer, @tgorochowik: This is an example of cleaning up and elaborating a particular patch from the #12984 pile. |
We mutual conflicts with definition of
struct timevalamong minlibs, newlib, and POSIX spec. This patchset fixes them.