[libcxx-commits] [PATCH] D93542: [SystemZ][ZOS] Provide CLOCK_MONOTONIC alternative

Hubert Tong via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jan 18 21:32:14 PST 2021


hubert.reinterpretcast added inline comments.


================
Comment at: libcxx/include/support/ibm/gettod.h:13-17
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
----------------
This seems to be a lot of header includes to get the definition of `timespec64`. I don't see a use of `assert` in the code.


================
Comment at: libcxx/include/support/ibm/gettod.h:37
+      return cc;
+   unsigned long long ms = (value.hi >> 4);
+   unsigned long long ns = ((value.hi & 0x0F) << 8) + (value.lo >> 56);
----------------
Please do not name a variable representing microseconds as `ms`. Use `us` (see `std::chrono_literals`).


================
Comment at: libcxx/include/support/ibm/gettod.h:41
+   ms = ms - 2208988800000000;
+   output->tv_sec = ms / 1000000;
+   output->tv_nsec = (ms % 1000000) * 1000 + ns;
----------------
Would `lldiv` help here?


================
Comment at: libcxx/src/chrono.cpp:168
+  if (0 != __gettod(&ts))
+        __throw_system_error(errno, "failed to obtained time of day");
+
----------------
hubert.reinterpretcast wrote:
> 
I don't see where `errno` is populated to indicate the error represented by a non-zero condition code in the implementation of `__gettod`. Is it meaningful to use `errno` here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93542/new/

https://reviews.llvm.org/D93542



More information about the libcxx-commits mailing list