[SystemZ][z/OS] correct rc and errno within nanosleep()

[zibi]

Summary

This patch fixes rc and errno within nanosleep(). It also updates __rem parameter as well it introduces cast to handle conversions from long into unsigned int to avoid warnings.

[zibi]

Fix issues with rc and errno as well adding cast for conversion warrnings.

[zibi]

fixing CI failure

[Mordante]

Some minor issues, other than that LGTM.

libcxx/include/__support/ibm/nanosleep.h

41

rc -> __rc.

44

Should we add a comment this value might be too large?

[ldionne]

This commit message is highly misleading. This is rewriting the function, not only tuning down warning levels. Can you please delimit more clearly what your change is doing if you want a meaningful review? It might be a good idea to break it down into more than one patch, too.

[ldionne]

By the way, my comments are made in the spirit of encouraging good review practices more than anything. I otherwise don't have a stake in this patch, since this is z/OS which is not officially supported anyway (since it lacks CI).

[zibi]

addressing comments

[zibi]

In D99373#2662527, @ldionne wrote:

This commit message is highly misleading.

Sorry about that, I fixed the description so it should be clear now.

[Mordante]

libcxx/include/__support/ibm/nanosleep.h

35

Please use complete sentences starting with a capital and ending with a full stop.

45

I think I wasn't clear enough regarding what I meant with too large.
When nanosleep fails with EINTR it should set the remaining time in __rem.
When it fails during sleep it sets the proper value (except rounding up).
When it fails during usleep with EINTR it acts as if it slept for 0 µs, which might be wrong. AFAIK it's not possible to fix this unless usleep on z/OS provides more information.

So I like the mentioning of the rounding up, but I would also like some information regarding ignoring the time slept.

[zibi]

adding short comments about implementation

[zibi]

libcxx/include/__support/ibm/nanosleep.h

45

Thanks, have a look at new comments.

[Mordante]

Thanks for the update. LGTM!

[curdeius]

libcxx/include/__support/ibm/nanosleep.h

45

Typo. But this comment sounds a bit awkward to me.

[zibi]

libcxx/include/__support/ibm/nanosleep.h

45

Thx for catching this. I will rewrite the comment and land this patch.

[ldionne]

@zibi Please do *not* commit patches when the libcxx review group is not green. The policy for committing to libc++ is clear - you have to wait for the libcxx group to be green, or you will get reverted.

This is OK for now and I'm sure you did not intend to do wrong, but please follow the process in the future.

[zibi]

In D99373#2677776, @ldionne wrote:

@zibi Please do *not* commit patches when the libcxx review group is not green. The policy for committing to libc++ is clear - you have to wait for the libcxx group to be green, or you will get reverted.

This is OK for now and I'm sure you did not intend to do wrong, but please follow the process in the future.

@ldionne Yes, sorry about that, I'm still learning the process.
The last commit was [NFC] so I thought it will be fine but will wait in the future and thank you for brining this up to my attention.

libcxx/include/__support/ibm/nanosleep.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP_SUPPORT_IBM_NANOSLEEP_H
 #define _LIBCPP_SUPPORT_IBM_NANOSLEEP_H
 
 #include <unistd.h>
 
-inline int nanosleep(const struct timespec* req, struct timespec* rem)
-{
-   // The nanosleep() function is not available on z/OS. Therefore, we will call
-   // sleep() to sleep for whole seconds and usleep() to sleep for any remaining
-   // fraction of a second. Any remaining nanoseconds will round up to the next
-   // microsecond.
-
-   useconds_t __micro_sec = (rem->tv_nsec + 999) / 1000;
-   if (__micro_sec > 999999)
-   {
-     ++rem->tv_sec;
-     __micro_sec -= 1000000;
-   }
-   while (rem->tv_sec)
-      rem->tv_sec = sleep(rem->tv_sec);
-   if (__micro_sec) {
-     rem->tv_nsec = __micro_sec * 1000;
-     return usleep(__micro_sec);
-   }
-   rem->tv_nsec = 0;
-   return 0;
+inline int nanosleep(const struct timespec* __req, struct timespec* __rem) {
+  if (__req->tv_sec < 0 || __req->tv_nsec < 0 || __req->tv_nsec > 999999999) {
+    errno = EINVAL;
+    return -1;
+  }
+  useconds_t __micro_sec =
+      static_cast<useconds_t>((__req->tv_nsec + 999) / 1000);
+  time_t __sec = __req->tv_sec;
+  if (__micro_sec > 999999) {
+    ++__sec;
+  __sec = sleep(static_cast<unsigned int>(__sec));
+  if (__sec) {
+    if (__rem) {
+      __rem->tv_sec = __sec;
+      __rem->tv_nsec = __micro_sec * 1000;
+    }
+    errno = EINTR;
+    return -1;
+  }
+    int rc = usleep(__micro_sec);
+    if (rc != 0 && __rem) {
+      __rem->tv_sec = 0;
+      __rem->tv_nsec = __micro_sec * 1000;
+      // errno is already set
+      return -1;
+    }
+    return rc;
 }
 
 #endif // _LIBCPP_SUPPORT_IBM_NANOSLEEP_H