[WIP] posix: remove zephyr posix headers from include path #97152
+2,490
−1,370
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cfriedt
force-pushed
the
remove-include-zephyr-posix-from-search-path
branch
28 times, most recently
from
986f71b to
2f6295b
Compare
The eventfd configuration does not need to be so deeply nested within POSIX since it does not depend on POSIX completely. Signed-off-by: Chris Friedt <[email protected]>
Separate the POSIX implementation into two categories: - Extensions to ISO C - System Interfaces The first category include standalone functions that generally do not require OS support or depend on any other features within the POSIX specification. The Option Groups that comprise this category include - POSIX_C_LIB_EXT: e.g. strnlen(), fnmatch() - POSIX_C_LANG_SUPPORT_R: e.g. gmtime_r(), strtok_r() The second category includes the majority of other POSIX Option Groups that do require OS support. The latter group may also be categorized generally as being NATIVE_LIBC_INCOMPATIBLE, although that might eventually become more granular. Signed-off-by: Chris Friedt <[email protected]>
The getopt sources were not formatted according to our codying style conventions, so apply formatting rules as a separate commit. Signed-off-by: Chris Friedt <[email protected]>
Add a custom Zephyr POSIX subprofile specifically for enabling the default features that Zephyr requires as per the coding guidelines. Signed-off-by: Chris Friedt <[email protected]>
Default POSIX_AEP_CHOICE to POSIX_AEP_CHOICE_ZEPHYR Signed-off-by: Chris Friedt <[email protected]>
Add zephyr-keep-sorted-start and zephyr-keep-sorted-stop comments. Signed-off-by: Chris Friedt <[email protected]>
The `sys/types.h` header is part of POSIX, which is optional in Zephyr and is mostly not implemented by Zephyr's minimal C library. Additionally, the only reason `sys/types.h` is included in `string.c` (in the minimal libc) is because of the non-standard `mem_word_t` type. `mem_word_t` itself is just a clone of `uintptr_t`, which is a standard type, so use the standard type, and reduce the amount of POSIX header leakage. `mem_word_t` is not used anywhere else in-tree or in modules hosted by the Zephyr project. ```shell $ cd ~/zephyrproject; grep -Rn "mem_word_t" * 2>/dev/null $ ``` Signed-off-by: Chris Friedt <[email protected]>
Previously, `time_t` was only checked in some C libraries to be greater than or equal to a 64-bit integer and it was skipped in other C libraries. The C standard still does not make this guarantee, and other code within Zephyr already has been changed to accomodate both 32-bit `time_t` and 64-bit `time_t`. There really is no point in having this test, since it is inconsistent at best. Until Zephyr changes to a 64-bit `time_t` (e.g. `time64_t`), then there is no guarantee of safety against the Y2038 problem. Signed-off-by: Chris Friedt <[email protected]>
The `sched_yield()` function was originally included to facilitate the of POSIX_REALTIME and POSIX_THREADS_EXT Option Groups in Issue 5. It was then marked as part of the _POSIX_PROCESS_SCHEDULING Option in Issue 6, but then was not clearly marked as part of the POSIX_THREADS(_BASE) Option Group until Issue 7. Moving it to `pthread.c` (and making it a function with regular linkage rather than inline) ensures that it will be available with the `POSIX_THREADS` Option Group. For more information, please see `POSIX_THREADS_BASE` in https://pubs.opengroup.org/onlinepubs/9699919799/xrat/\ V4_subprofiles.html and https://pubs.opengroup.org/onlinepubs/9799919799/xrat/\ V4_subprofiles.html Signed-off-by: Chris Friedt <[email protected]>
The net capture sample is likely one application that does rely on linking
against the native C library rather than Picolibc, since it needs to hook
into Linux's tun / tap interfaces.
However, after removing `<zephyr/posix/...h>` from the default search
path, a number of conflicts appear between the native libc socket types
and Zephyr's in `net_ip.h`.
Issue 97050 already captures this partially. However, there seems to be
some confusion about POSIX headers and types and ISO C headers and types.
There are C libraries that do not include all of the necessary POSIX
headers and types that Zephyr depends on. For example:
* minimal libc
* IAR
* newlib
* picolibc
The latter two might surprise some.
In any case, the work required to properly namespace `net_ip.h` constants
and types is well out of the scope of the current release cycles, so this
small workaround should suffice.
A non-exhaustive list of constants and types from `net_ip.h` that conflict
with native types are
* PF_INET
* PF_INET6
* PF_PACKET
* PF_CAN
* PF_LOCAL
* SOCK_STREAM
* SOCK_DGRAM
* SOCK_RAW
* struct iovec
* struct msghdr
* struct cmsghdr
* struct sockaddr
* struct sockaddr_storage
A non-exhaustive list of errors that arise are of the form(s) below:
```shell
In file included from $HOME/../zephyr/net/ethernet.h:21,
from $HOME/../samples/net/capture/src/main.c:15:
../zephyr/net/net_ip.h:45: warning: "PF_INET" redefined
45 | #define PF_INET 1 /**< IP protocol family version 4. */
|
/usr/include/bits/socket.h:45: note: this is the location of the previous \
definition
45 | #define PF_INET 2 /* IP protocol family. */
|
In file included from /usr/include/bits/socket.h:38:
$HOME/zephyrproject/zephyr/include/zephyr/net/net_ip.h:89:9: error: \
redeclaration of enumerator ‘SOCK_STREAM’
89 | SOCK_STREAM = 1, /**< Stream socket type */
| ^~~~~~~~~~~
/usr/include/bits/socket_type.h:26:3: note: previous definition of \
‘SOCK_STREAM’ with type ‘enum __socket_type’
26 | SOCK_STREAM = 1, /* Sequenced, reliable,...
| ^~~~~~~~~~~
$HOME/zephyrproject/zephyr/include/zephyr/net/net_ip.h:250:8: error: \
redefinition of ‘struct iovec’
250 | struct iovec {
| ^~~~~
In file included from /usr/include/sys/socket.h:26:
/usr/include/bits/types/struct_iovec.h:26:8: note: originally defined here
26 | struct iovec
| ^~~~~
```
Signed-off-by: Chris Friedt <[email protected]>
cfriedt
force-pushed
the
remove-include-zephyr-posix-from-search-path
branch
from
2f6295b to
3a5d01c
Compare
Indicate that C library headers have declared `struct sigevent` and `struct sigval` to avoid redefinition warnings / errors. Signed-off-by: Chris Friedt <[email protected]>
In order to avoid multiple definition errors, indicate that struct timespec is declared. Signed-off-by: Chris Friedt <[email protected]>
In order to avoid multiple definition errors, indicate that struct timeval is declared. Signed-off-by: Chris Friedt <[email protected]>
Avoid redefining `time_t` by always including the definition from `<time.h>`, since the C library may use something other than `long`. This is still safe according to the specification, since it explicitly states: > Inclusion of the <signal.h> header may make visible all symbols from > the <time.h> header. For more information, please see https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html Signed-off-by: Chris Friedt <[email protected]>
C11 requires `<time.h>` to define `struct timespec`, so do not define it when C11 is already known. Signed-off-by: Chris Friedt <[email protected]>
A previous typo went undetected that declared a type-defined `timespec_t`, which is obviously incorrect, since it is only specified as `struct timespec`. Signed-off-by: Chris Friedt <[email protected]>
A previous typo went undetected that declared a type-defined `sigevent_t`, which is obviously incorrect, since it is only specified as `struct sigevent`. Signed-off-by: Chris Friedt <[email protected]>
In order to avoid warnings, declare `union sigval` ahead of `struct sigevent` and declare `siginfo_t` ahead of `struct sigaction`. Signed-off-by: Chris Friedt <[email protected]>
Previously, the `sigev_notify_function` field was missing from `struct sigevent`. Additionally, `sigev_notify_attributes` were incorrectly named `sigev_thread_attr`. Add it back to ensure that we are able to support realtime signals. Signed-off-by: Chris Friedt <[email protected]>
If C libraries provide conformant POSIX headers, then use the headers provided by the C library. Otherwise, C library maintainers may add `include/zephyr/posix` to their standard search path. Signed-off-by: Chris Friedt <[email protected]>
WIP Signed-off-by: Chris Friedt <[email protected]>
cfriedt
force-pushed
the
remove-include-zephyr-posix-from-search-path
branch
from
3a5d01c to
feb309a
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change means that we no longer need to have
include/zephyr/posixin the library search path for C libraries that have (at least mostly) conformant POSIX headers. In other words, we use the C library's POSIX headers.What is nice about this is that it
#include_next <foo.h>CONFIG_POSIX_API(follow-up PR)... probably a lot more
The main commits from this PR are:
<zephyr/posix/...h>to<...h>Dependent changes broken-out to separate PRs: