Skip to content

Commit 2d0c9e2

Browse files
authored
Cleanup time_unix.c (ros2#389)
The file was pretty messy. Cleanups: 1. Remove totally unnecessary #ifdef __cplusplus; that is only necessary in headers. 2. Remove unnecessary include to ./common.h. 3. Change WOULD_BE_NEGATIVE to an inline function, which will at least give us some type checking. 4. Get rid of unnecessary duplicated code for macOS. Most of the time the code was exactly the same. 5. Make sure to check the return value from clock_gettime. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
1 parent 94b3018 commit 2d0c9e2

1 file changed

Lines changed: 24 additions & 29 deletions

File tree

src/time_unix.c

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@
1616
# error time_unix.c is not intended to be used with win32 based systems
1717
#endif // defined(_WIN32)
1818

19-
#ifdef __cplusplus
20-
extern "C"
21-
{
22-
#endif
23-
2419
#include "rcutils/time.h"
2520

2621
#if defined(__MACH__) && defined(__APPLE__)
@@ -40,36 +35,38 @@ extern "C"
4035
#include <time.h>
4136
#endif // defined(__ZEPHYR__)
4237

38+
#include <errno.h>
4339
#include <unistd.h>
44-
#include "./common.h"
40+
4541
#include "rcutils/allocator.h"
4642
#include "rcutils/error_handling.h"
4743

4844
#if !defined(__MACH__) && !defined(__APPLE__) // Assume clock_get_time is available on OS X.
49-
// This id an appropriate check for clock_gettime() according to:
45+
// This is an appropriate check for clock_gettime() according to:
5046
// http://man7.org/linux/man-pages/man2/clock_gettime.2.html
5147
# if !defined(_POSIX_TIMERS) || !_POSIX_TIMERS
5248
# error no monotonic clock function available
5349
# endif // !defined(_POSIX_TIMERS) || !_POSIX_TIMERS
5450
#endif // !defined(__MACH__) && !defined(__APPLE__)
5551

56-
#define __WOULD_BE_NEGATIVE(seconds, subseconds) (seconds < 0 || (subseconds < 0 && seconds == 0))
52+
static inline bool would_be_negative(const struct timespec * const now)
53+
{
54+
return now->tv_sec < 0 || (now->tv_nsec < 0 && now->tv_sec == 0);
55+
}
5756

5857
rcutils_ret_t
5958
rcutils_system_time_now(rcutils_time_point_value_t * now)
6059
{
6160
RCUTILS_CHECK_ARGUMENT_FOR_NULL(now, RCUTILS_RET_INVALID_ARGUMENT);
6261
struct timespec timespec_now;
63-
#if defined(__MACH__) && defined(__APPLE__)
64-
// On macOS, use clock_gettime(CLOCK_REALTIME), which matches
65-
// the clang implementation
62+
// Using clock_gettime(CLOCK_REALTIME) matches what both Linux and macOS use.
63+
// For macOS, see the clang implementation at
6664
// (https://github.com/llvm/llvm-project/blob/baebe12ad0d6f514cd33e418d6504075d3e79c0a/libcxx/src/chrono.cpp)
67-
clock_gettime(CLOCK_REALTIME, &timespec_now);
68-
#else // defined(__MACH__) && defined(__APPLE__)
69-
// Otherwise use clock_gettime.
70-
clock_gettime(CLOCK_REALTIME, &timespec_now);
71-
#endif // defined(__MACH__) && defined(__APPLE__)
72-
if (__WOULD_BE_NEGATIVE(timespec_now.tv_sec, timespec_now.tv_nsec)) {
65+
if (clock_gettime(CLOCK_REALTIME, &timespec_now) < 0) {
66+
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("Failed to get system time: %d", errno);
67+
return RCUTILS_RET_ERROR;
68+
}
69+
if (would_be_negative(&timespec_now)) {
7370
RCUTILS_SET_ERROR_MSG("unexpected negative time");
7471
return RCUTILS_RET_ERROR;
7572
}
@@ -81,25 +78,23 @@ rcutils_ret_t
8178
rcutils_steady_time_now(rcutils_time_point_value_t * now)
8279
{
8380
RCUTILS_CHECK_ARGUMENT_FOR_NULL(now, RCUTILS_RET_INVALID_ARGUMENT);
84-
// If clock_gettime is available or on OS X, use a timespec.
8581
struct timespec timespec_now;
82+
clockid_t monotonic_clock = CLOCK_MONOTONIC;
83+
8684
#if defined(__MACH__) && defined(__APPLE__)
87-
// On macOS, use clock_gettime(CLOCK_MONOTONIC_RAW), which matches
88-
// the clang implementation
85+
// On macOS, use CLOCK_MONOTONIC_RAW, which matches the clang implementation
8986
// (https://github.com/llvm/llvm-project/blob/baebe12ad0d6f514cd33e418d6504075d3e79c0a/libcxx/src/chrono.cpp)
90-
clock_gettime(CLOCK_MONOTONIC_RAW, &timespec_now);
91-
#else // defined(__MACH__) && defined(__APPLE__)
92-
// Otherwise use clock_gettime.
93-
clock_gettime(CLOCK_MONOTONIC, &timespec_now);
87+
monotonic_clock = CLOCK_MONOTONIC_RAW;
9488
#endif // defined(__MACH__) && defined(__APPLE__)
95-
if (__WOULD_BE_NEGATIVE(timespec_now.tv_sec, timespec_now.tv_nsec)) {
89+
90+
if (clock_gettime(monotonic_clock, &timespec_now) < 0) {
91+
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("Failed to get steady time: %d", errno);
92+
return RCUTILS_RET_ERROR;
93+
}
94+
if (would_be_negative(&timespec_now)) {
9695
RCUTILS_SET_ERROR_MSG("unexpected negative time");
9796
return RCUTILS_RET_ERROR;
9897
}
9998
*now = RCUTILS_S_TO_NS((int64_t)timespec_now.tv_sec) + timespec_now.tv_nsec;
10099
return RCUTILS_RET_OK;
101100
}
102-
103-
#ifdef __cplusplus
104-
}
105-
#endif

0 commit comments

Comments
 (0)