lib: posix: remove posix/time.h

[cfriedt]

The <time.h> header typically belongs to the C libary. There is no reason for the posix subsystem to partially provide it.

Removing posix/time.h also mitigates the need for the #include_next workaround.

Compliance failure is a false positive.

[cfriedt]

Likely need to rework this with configuration bits for pico libc. Related: picolibc/picolibc#305

[keith-packard]

If you want to elide the libc wrapper, you should be able to define clock_gettime directly as a 'gnu_inline' function (where supported, which means gcc or clang). That might be a nice optimization.

extern __inline int __attribute ((gnu_inline, always_inline)) clock_gettime(clock_id_t clock_id, struct timespec *ts) 
{
    return zephyr_clock_gettime(clock_id, ts);
}

that would allow the C library 'time.h' header to continue to declare clock_gettime as a regular function but all uses will be inlined to direct calls to the zephyr syscall version.

[cfriedt]

Oh wow - that is quite a useful optimization. I worry though that there may be portability concerns.

[keith-packard]

This is looking pretty good. I think as a transition, you should add the necessary _POSIX definitions before the libc headers are included; that way newlib and picolibc will provide declarations for all of the necessary functions.

[cfriedt]

This is looking pretty good. I think as a transition, you should add the necessary _POSIX definitions before the libc headers are included; that way newlib and picolibc will provide declarations for all of the necessary functions.

I guess it could be done in an additional header that gets included with -include foo.h rather than command line switches. Will spin up another rev shortly.

[stephanosio]

In general, I think this should be accompanied by zephyrproject-rtos/sdk-ng#350, such that, for newlib and picolibc (included as part of the toolchains), when building arch-*-zephyr targets, the proper Zephyr-specific definitions are provided.

[cfriedt]

• Compliance warnings are a false positive
• likely / unlikely redefinition fixed by west.yml: update hal_espressif to fix redefinition issue #49928

[cfriedt]

I'm a bit concerned about this inconsistence - in Zephyr, this should technically be <zephyr/posix/signal.h> but iirc that breaks a couple of tests (if not here, then in another place where <signal.h> is used. IIRC, timer.c).

[cfriedt]

In general, I think this should be accompanied by zephyrproject-rtos/sdk-ng#350, such that, for newlib and picolibc (included as part of the toolchains), when building arch-*-zephyr targets, the proper Zephyr-specific definitions are provided.

@stephanosio - I would like to try and get this upstreamed sooner rather than later since it blocks my other longstanding PR that allows us to opt out of the <zephyr/posix/..> prefix for standard POSIX headers.

It would be good to sync up w.r.t. zephyrproject-rtos/sdk-ng#350 though. When do you think it will be ready? Is it possible this could go in first?

[yashi]

Having a problem using clock_gettime() with Zephyr 3.2 and found this PR.

Reading this and picolibc/picolibc#305 together but still not sure how this transition suppose to work. Zephyr has system call wrappers but we are moving them to picolibc? Is #49887 (comment) the answer?

What is the point of moving thin wrappers to picolibc or the libcs we support for that matter? Just like Glibc? Or am I missing the whole point? Really appreciate if someone enlighten me.
Thanks.

[yashi]

Where can I find this file? Is it syscalls/time.h?

[cfriedt]

it's generated dynamically by the build system.
See https://docs.zephyrproject.org/latest/kernel/usermode/syscalls.html

[yashi]

BTW, this is what I got with the two patches

In file included from ws/zephyr/include/zephyr/posix/time_calls.h:24,
                 from ws/zephyr/lib/posix/clock.c:10:
ws/build/zephyr/include/generated/syscalls/time.h:24:19: error: static declaration of 'clock_gettime' follows non-static declaration
   24 | static inline int clock_gettime(clockid_t clock_id, struct timespec * ts)
      |                   ^~~~~~~~~~~~~
In file included from ws/zephyr/lib/posix/clock.c:8:
ws/build/modules/picolibc/picolibc/include/time.h:212:5: note: previous declaration of 'clock_gettime' with type 'int(clockid_t,  struct timespec *)' {aka 'int(long unsigned int,  struct timespec *)'}
  212 | int clock_gettime (clockid_t clock_id, struct timespec *tp);
      |     ^~~~~~~~~~~~~
ws/zephyr/lib/posix/clock.c:78:5: error: redefinition of 'clock_gettime'
   78 | int clock_gettime(clockid_t clock_id, struct timespec *tp)
      |     ^~~~~~~~~~~~~
ws/build/zephyr/include/generated/syscalls/time.h:24:19: note: previous definition of 'clock_gettime' with type 'int(clockid_t,  struct timespec *)' {aka 'int(long unsigned int,  struct timespec *)'}
   24 | static inline int clock_gettime(clockid_t clock_id, struct timespec * ts)
      |                   ^~~~~~~~~~~~~
ws/zephyr/include/zephyr/posix/time_calls.h:17:15: warning: 'zephyr_clock_gettime' used but never defined
   17 | __syscall int zephyr_clock_gettime(clockid_t clock_id, struct timespec *ts);
      |               ^~~~~~~~~~~~~~~~~~~~
ninja: build stopped: subcommand failed.

[yashi]

https://discord.com/channels/720317445772017664/1016325697301659728

[cfriedt]

Hi @yashi - yeah, this PR is still in draft because it's part of a bigger effort to improve libc header consistency. We're aiming to go for newlib-compatible for the most part. This is on hold until some more work is done on zephyrproject-rtos/sdk-ng#350

[keith-packard]

In file included from ws/zephyr/include/zephyr/posix/time_calls.h:24,
from ws/zephyr/lib/posix/clock.c:10:
ws/build/zephyr/include/generated/syscalls/time.h:24:19: error: static declaration of 'clock_gettime' follows non-static declaration

This can be fixed by making sure the static inline definitions appear before the library declarations when compiling the code. All that would be required to make this work with libc is to ensure a non-static definition of these functions existed in case the C library itself called them, or to compile the C library against the Zephyr header files with the static inline definitions so that it would also use the static inline versions. Having the POSIX API be static inlines atop the Zephyr API would mean POSIX code ported to Zephyr would not incur any performance penalty from using regular POSIX names, so it seems worth a bit of effort to make that work.