[libc-commits] [PATCH] D134095: Implement nanosleep per https://pubs.opengroup.org/onlinepubs/009695399/basedefs/time.h.html

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Mon Sep 19 10:27:16 PDT 2022

sivachandra accepted this revision.
sivachandra added inline comments.
This revision is now accepted and ready to land.

Comment at: libc/include/llvm-libc-types/CMakeLists.txt:38
 add_header(struct_tm HDR struct_tm.h)
+add_header(struct_timespec HDR struct_timespec.h)
 add_header(thrd_start_t HDR thrd_start_t.h)
Since this depends on `time_t.h`, we should add a `DEPENDS .time_t`.

Comment at: libc/include/llvm-libc-types/struct_timespec.h:17
+  time_t tv_sec; /* Seconds.  */
+  long tv_nsec;  /* Nanoseconds.  */
I think this definition is correct as per the POSIX spec. But, since we are passing structures of this shape to the Linux syscall, we should probably add a comment or TODO in `src/time/nanosleep.cpp` explaining why this definition is sufficient or incomplete.

Comment at: libc/spec/stdc.td:17
Can you do the changes in the file separately as an obvious change?

Comment at: libc/src/time/mktime.h:1
 //===-- Implementation header of mktime -------------------------*- C++ -*-===//
Can you make the changes in this file also separately?

Comment at: libc/test/src/time/nanosleep_test.cpp:21
+TEST(LlvmLibcNanosleep, sleep) {
+  using __llvm_libc::testing::ErrnoSetterMatcher::Succeeds;
Call it `SmokeTest` may be? Also, move the above comment inside this test block.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list