common/ceph_time: Don't define public things in time_detail

Defining things in a detail section and then using them outside turned out to not be the best idea. Signed-off-by: Adam C. Emerson <[email protected]>
areed · Sep 15, 2020 · 01f706c · 01f706c
1 parent d94f226
commit 01f706c
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 72 deletions.
diff --git a/src/common/ceph_time.cc b/src/common/ceph_time.cc
@@ -55,11 +55,12 @@ int clock_gettime(int clk_id, struct timespec *tp)
 using namespace std::literals;
 
 namespace ceph {
-namespace time_detail {
+using std::chrono::seconds;
+using std::chrono::nanoseconds;
 void real_clock::to_ceph_timespec(const time_point& t,
 				  struct ceph_timespec& ts) {
   ts.tv_sec = to_time_t(t);
-  ts.tv_nsec = (t.time_since_epoch() % seconds(1)).count();
+  ts.tv_nsec = (t.time_since_epoch() % 1s).count();
 }
 struct ceph_timespec real_clock::to_ceph_timespec(const time_point& t) {
   struct ceph_timespec ts;
@@ -87,7 +88,6 @@ coarse_real_clock::time_point coarse_real_clock::from_ceph_timespec(
   return time_point(seconds(ts.tv_sec) + nanoseconds(ts.tv_nsec));
 }
 
-}
 
 using std::chrono::duration_cast;
 using std::chrono::seconds;

diff --git a/src/common/ceph_time.h b/src/common/ceph_time.h
@@ -21,8 +21,6 @@
 #include <optional>
 #include <sys/time.h>
 
-#include "include/ceph_assert.h"
-
 #if defined(__APPLE__)
 #include <sys/_types/_timespec.h>
 
@@ -35,11 +33,6 @@ int clock_gettime(int clk_id, struct timespec *tp);
 struct ceph_timespec;
 
 namespace ceph {
-namespace time_detail {
-using std::chrono::duration_cast;
-using std::chrono::seconds;
-using std::chrono::microseconds;
-using std::chrono::nanoseconds;
 // Currently we use a 64-bit count of nanoseconds.
 
 // We could, if we wished, use a struct holding a uint64_t count
@@ -49,13 +42,20 @@ using std::chrono::nanoseconds;
 // want.
 typedef uint64_t rep;
 
-// A concrete duration, unsigned. The timespan Ceph thinks in.
+
+// duration is the concrete time representation for our code in the
+// case that we are only interested in durations between now and the
+// future. Using it means we don't have to have EVERY function that
+// deals with a duration be a template. We can do so for user-facing
+// APIs, however.
 typedef std::chrono::duration<rep, std::nano> timespan;
 
 
 // Like the above but signed.
 typedef int64_t signed_rep;
 
+// Similar to the above but for durations that can specify
+// differences between now and a time point in the past.
 typedef std::chrono::duration<signed_rep, std::nano> signedspan;
 
 // We define our own clocks so we can have our choice of all time
@@ -67,6 +67,8 @@ typedef std::chrono::duration<signed_rep, std::nano> signedspan;
 // One potential issue is that we should accept system_clock
 // timepoints in user-facing APIs alongside (or instead of)
 // ceph::real_clock times.
+
+// High-resolution real-time clock
 class real_clock {
 public:
   typedef timespan duration;
@@ -97,34 +99,36 @@ class real_clock {
   static time_point to_system_time_point(
     const std::chrono::time_point<Clock, Duration>& t) {
     return time_point(seconds(Clock::to_time_t(t)) +
-		      duration_cast<duration>(t.time_since_epoch() %
-					      seconds(1)));
+		      std::chrono::duration_cast<duration>(t.time_since_epoch() %
+							   std::chrono::seconds(1)));
   }
   template<typename Clock, typename Duration>
   static std::chrono::time_point<Clock, Duration> to_system_time_point(
     const time_point& t) {
     return (Clock::from_time_t(to_time_t(t)) +
-	    duration_cast<Duration>(t.time_since_epoch() % seconds(1)));
+	    std::chrono::duration_cast<Duration>(t.time_since_epoch() %
+						 std::chrono::seconds(1)));
   }
 
   static time_t to_time_t(const time_point& t) noexcept {
-    return duration_cast<seconds>(t.time_since_epoch()).count();
+    return std::chrono::duration_cast<std::chrono::seconds>(t.time_since_epoch()).count();
   }
   static time_point from_time_t(const time_t& t) noexcept {
-    return time_point(seconds(t));
+    return time_point(std::chrono::seconds(t));
   }
 
   static void to_timespec(const time_point& t, struct timespec& ts) {
     ts.tv_sec = to_time_t(t);
-    ts.tv_nsec = (t.time_since_epoch() % seconds(1)).count();
+    ts.tv_nsec = (t.time_since_epoch() % std::chrono::seconds(1)).count();
   }
   static struct timespec to_timespec(const time_point& t) {
     struct timespec ts;
     to_timespec(t, ts);
     return ts;
   }
   static time_point from_timespec(const struct timespec& ts) {
-    return time_point(seconds(ts.tv_sec) + nanoseconds(ts.tv_nsec));
+    return time_point(std::chrono::seconds(ts.tv_sec) +
+		      std::chrono::nanoseconds(ts.tv_nsec));
   }
 
   static void to_ceph_timespec(const time_point& t,
@@ -134,27 +138,29 @@ class real_clock {
 
   static void to_timeval(const time_point& t, struct timeval& tv) {
     tv.tv_sec = to_time_t(t);
-    tv.tv_usec = duration_cast<microseconds>(t.time_since_epoch() %
-					     seconds(1)).count();
+    tv.tv_usec = std::chrono::duration_cast<std::chrono::microseconds>(
+      t.time_since_epoch() % std::chrono::seconds(1)).count();
   }
   static struct timeval to_timeval(const time_point& t) {
     struct timeval tv;
     to_timeval(t, tv);
     return tv;
   }
   static time_point from_timeval(const struct timeval& tv) {
-    return time_point(seconds(tv.tv_sec) + microseconds(tv.tv_usec));
+    return time_point(std::chrono::seconds(tv.tv_sec) +
+		      std::chrono::microseconds(tv.tv_usec));
   }
 
   static double to_double(const time_point& t) {
     return std::chrono::duration<double>(t.time_since_epoch()).count();
   }
   static time_point from_double(const double d) {
-    return time_point(duration_cast<duration>(
+    return time_point(std::chrono::duration_cast<duration>(
 			std::chrono::duration<double>(d)));
   }
 };
 
+// Low-resolution but preusmably faster real-time clock
 class coarse_real_clock {
 public:
   typedef timespan duration;
@@ -191,23 +197,25 @@ class coarse_real_clock {
   }
 
   static time_t to_time_t(const time_point& t) noexcept {
-    return duration_cast<seconds>(t.time_since_epoch()).count();
+    return std::chrono::duration_cast<std::chrono::seconds>(
+      t.time_since_epoch()).count();
   }
   static time_point from_time_t(const time_t t) noexcept {
-    return time_point(seconds(t));
+    return time_point(std::chrono::seconds(t));
   }
 
   static void to_timespec(const time_point& t, struct timespec& ts) {
     ts.tv_sec = to_time_t(t);
-    ts.tv_nsec = (t.time_since_epoch() % seconds(1)).count();
+    ts.tv_nsec = (t.time_since_epoch() % std::chrono::seconds(1)).count();
   }
   static struct timespec to_timespec(const time_point& t) {
     struct timespec ts;
     to_timespec(t, ts);
     return ts;
   }
   static time_point from_timespec(const struct timespec& ts) {
-    return time_point(seconds(ts.tv_sec) + nanoseconds(ts.tv_nsec));
+    return time_point(std::chrono::seconds(ts.tv_sec) +
+		      std::chrono::nanoseconds(ts.tv_nsec));
   }
 
   static void to_ceph_timespec(const time_point& t,
@@ -217,27 +225,29 @@ class coarse_real_clock {
 
   static void to_timeval(const time_point& t, struct timeval& tv) {
     tv.tv_sec = to_time_t(t);
-    tv.tv_usec = duration_cast<microseconds>(t.time_since_epoch() %
-					     seconds(1)).count();
+    tv.tv_usec = std::chrono::duration_cast<std::chrono::microseconds>(
+      t.time_since_epoch() % std::chrono::seconds(1)).count();
   }
   static struct timeval to_timeval(const time_point& t) {
     struct timeval tv;
     to_timeval(t, tv);
     return tv;
   }
   static time_point from_timeval(const struct timeval& tv) {
-    return time_point(seconds(tv.tv_sec) + microseconds(tv.tv_usec));
+    return time_point(std::chrono::seconds(tv.tv_sec) +
+		      std::chrono::microseconds(tv.tv_usec));
   }
 
   static double to_double(const time_point& t) {
     return std::chrono::duration<double>(t.time_since_epoch()).count();
   }
   static time_point from_double(const double d) {
-    return time_point(duration_cast<duration>(
+    return time_point(std::chrono::duration_cast<duration>(
 			std::chrono::duration<double>(d)));
   }
 };
 
+// High-resolution monotonic clock
 class mono_clock {
 public:
   typedef timespan duration;
@@ -249,7 +259,8 @@ class mono_clock {
   static time_point now() noexcept {
     struct timespec ts;
     clock_gettime(CLOCK_MONOTONIC, &ts);
-    return time_point(seconds(ts.tv_sec) + nanoseconds(ts.tv_nsec));
+    return time_point(std::chrono::seconds(ts.tv_sec) +
+		      std::chrono::nanoseconds(ts.tv_nsec));
   }
 
   static bool is_zero(const time_point& t) {
@@ -259,12 +270,10 @@ class mono_clock {
   static time_point zero() {
     return time_point::min();
   }
-
-  // A monotonic clock's timepoints are only meaningful to the
-  // computer on which they were generated. Thus having an
-  // optional skew is meaningless.
 };
 
+// Low-resolution but, I would hope or there's no point, faster
+// monotonic clock
 class coarse_mono_clock {
 public:
   typedef timespan duration;
@@ -287,7 +296,8 @@ class coarse_mono_clock {
 #warning Falling back to CLOCK_MONOTONIC, may be slow.
     clock_gettime(CLOCK_MONOTONIC, &ts);
 #endif
-    return time_point(seconds(ts.tv_sec) + nanoseconds(ts.tv_nsec));
+    return time_point(std::chrono::seconds(ts.tv_sec) +
+		      std::chrono::nanoseconds(ts.tv_nsec));
   }
 
   static bool is_zero(const time_point& t) {
@@ -299,9 +309,9 @@ class coarse_mono_clock {
   }
 };
 
+namespace time_detail {
 // So that our subtractions produce negative spans rather than
 // arithmetic underflow.
-namespace {
 template<typename Rep1, typename Period1, typename Rep2,
 	 typename Period2>
 inline auto difference(std::chrono::duration<Rep1, Period1> minuend,
@@ -336,32 +346,6 @@ inline auto difference(
 		    subtrahend.time_since_epoch());
 }
 }
-} // namespace time_detail
-
-  // duration is the concrete time representation for our code in the
-  // case that we are only interested in durations between now and the
-  // future. Using it means we don't have to have EVERY function that
-  // deals with a duration be a template. We can do so for user-facing
-  // APIs, however.
-using time_detail::timespan;
-
-// Similar to the above but for durations that can specify
-// differences between now and a time point in the past.
-using time_detail::signedspan;
-
-// High-resolution real-time clock
-using time_detail::real_clock;
-
-// Low-resolution but preusmably faster real-time clock
-using time_detail::coarse_real_clock;
-
-
-// High-resolution monotonic clock
-using time_detail::mono_clock;
-
-// Low-resolution but, I would hope or there's no point, faster
-// monotonic clock
-using time_detail::coarse_mono_clock;
 
 // Please note that the coarse clocks are disjoint. You cannot
 // subtract a real_clock timepoint from a coarse_real_clock
@@ -430,15 +414,13 @@ auto ceil(const std::chrono::time_point<Clock, Duration>& timepoint,
 	ceil(timepoint.time_since_epoch(), precision));
 }
 
-namespace {
 inline timespan make_timespan(const double d) {
   return std::chrono::duration_cast<timespan>(
     std::chrono::duration<double>(d));
 }
 inline std::optional<timespan> maybe_timespan(const double d) {
   return d ? std::make_optional(make_timespan(d)) : std::nullopt;
 }
-}
 
 std::ostream& operator<<(std::ostream& m, const timespan& t);
 template<typename Clock,
@@ -453,7 +435,6 @@ std::ostream& operator<<(std::ostream& m,
 // The way std::chrono handles the return type of subtraction is not
 // wonderful. The difference of two unsigned types SHOULD be signed.
 
-namespace {
 inline signedspan operator -(real_time minuend,
 			     real_time subtrahend) {
   return time_detail::difference(minuend, subtrahend);
@@ -473,13 +454,10 @@ inline signedspan operator -(coarse_mono_time minuend,
 			     coarse_mono_time subtrahend) {
   return time_detail::difference(minuend, subtrahend);
 }
-}
 
 // We could add specializations of time_point - duration and
 // time_point + duration to assert on overflow, but I don't think we
 // should.
-
-
 inline timespan abs(signedspan z) {
   return z > signedspan::zero() ?
     std::chrono::duration_cast<timespan>(z) :

diff --git a/src/common/iso_8601.cc b/src/common/iso_8601.cc
@@ -6,6 +6,7 @@
 
 #include "iso_8601.h"
 #include "include/timegm.h"
+#include "include/ceph_assert.h"
 
 namespace ceph {
 using std::chrono::duration_cast;

diff --git a/src/messages/MOSDPing.h b/src/messages/MOSDPing.h
@@ -64,7 +64,7 @@ class MOSDPing : public Message {
   utime_t ping_stamp;               ///< when the PING was sent
   ceph::signedspan mono_ping_stamp; ///< relative to sender's clock
   ceph::signedspan mono_send_stamp; ///< replier's send stamp
-  std::optional<ceph::time_detail::signedspan> delta_ub;  ///< ping sender
+  std::optional<ceph::signedspan> delta_ub;  ///< ping sender
   epoch_t up_from = 0;
 
   uint32_t min_message_size = 0;
@@ -75,7 +75,7 @@ class MOSDPing : public Message {
 	   ceph::signedspan mss,
 	   epoch_t upf,
 	   uint32_t min_message,
-	   std::optional<ceph::time_detail::signedspan> delta_ub = {})
+	   std::optional<ceph::signedspan> delta_ub = {})
     : Message{MSG_OSD_PING, HEAD_VERSION, COMPAT_VERSION},
       fsid(f), map_epoch(e), op(o),
       ping_stamp(s),

diff --git a/src/rgw/rgw_sync_error_repo.h b/src/rgw/rgw_sync_error_repo.h
@@ -23,7 +23,7 @@ class RGWCoroutine;
 struct rgw_raw_obj;
 
 // decode a timestamp as a uint64_t for CMPXATTR_MODE_U64
-ceph::real_time rgw_error_repo_decode_value(const bufferlist& bl);
+ceph::real_time rgw_error_repo_decode_value(const ceph::bufferlist& bl);
 
 // write an omap key iff the given timestamp is newer
 int rgw_error_repo_write(librados::ObjectWriteOperation& op,