[Git][NTPsec/ntpsec][master] Eliminate the ugly and dangerous global time-function pointer.
Eric S. Raymond
gitlab at mg.gitlab.com
Mon Nov 27 12:12:17 UTC 2017
Eric S. Raymond pushed to branch master at NTPsec / ntpsec
Commits:
1815910a by Eric S. Raymond at 2017-11-27T06:59:26-05:00
Eliminate the ugly and dangerous global time-function pointer.
This is a refactoring step; no behavior changes, and all unit tests pass.
But the way the pivot date used by various calendar functions is set is
changed. Used to be the pivot-setting operation was snuck in through the
back door using a global pointer to a time-setting function; if the pointer was
NULL various individual functions would call time(NULL). The reason for this
ugly hack was so time could be frozen in test jigs.
The new logic generates a pivot value as far up the call stack as possible
and passes it to where it's needed. There is one exception to this, in
libntp/prettydate.c, where a module static pivot is set by a new initialization
function; this needs to be called in ntpd's initialization logic.
- - - - -
18 changed files:
- include/ntp_calendar.h
- include/ntp_fp.h
- include/ntp_stdlib.h
- include/timespecops.h
- libntp/clocktime.c
- libntp/ntp_calendar.c
- libntp/prettydate.c
- libntp/pymodule.c
- libntp/systime.c
- libntp/timespecops.c
- ntpd/ntp_refclock.c
- ntpd/ntpd.c
- ntpd/refclock_nmea.c
- tests/common/caltime.c
- tests/common/caltime.h
- tests/libntp/clocktime.c
- tests/libntp/prettydate.c
- tests/ntpd/leapsec.c
Changes:
=====================================
include/ntp_calendar.h
=====================================
--- a/include/ntp_calendar.h
+++ b/include/ntp_calendar.h
@@ -29,14 +29,6 @@ typedef struct {
typedef time_t (*systime_func_ptr)(time_t *);
/*
- * set the function for getting the system time. This is mostly used for
- * unit testing to provide a fixed / shifted time stamp. Setting the
- * value to NULL restores the original function, that is, 'time()',
- * which is also the automatic default.
- */
-extern systime_func_ptr ntpcal_set_timefunc(systime_func_ptr);
-
-/*
* We deal in a 4 year cycle starting at March 1, 1900. We assume
* we will only want to deal with dates since then, and not to exceed
* the rollover day in 2036.
@@ -67,7 +59,7 @@ ntpcal_get_build_date(struct calendar * /* jd */);
* current system time.
*/
extern time64_t
-ntpcal_ntp_to_time(uint32_t /* ntp */, const time_t * /* pivot */);
+ntpcal_ntp_to_time(uint32_t /* ntp */, time_t /* pivot */);
/*
* Convert a timestamp in NTP scale to a 64bit seconds value in the NTP
@@ -76,7 +68,7 @@ ntpcal_ntp_to_time(uint32_t /* ntp */, const time_t * /* pivot */);
* Note: The pivot must be given in UN*X time scale!
*/
extern time64_t
-ntpcal_ntp_to_ntp(uint32_t /* ntp */, const time_t * /* pivot */);
+ntpcal_ntp_to_ntp(uint32_t /* ntp */, time_t /* pivot */);
/*
* Split a time stamp in seconds into elapsed days and elapsed seconds
@@ -187,7 +179,7 @@ ntpcal_ntp64_to_date(struct calendar * /* jd */, const time64_t /* ntp */);
extern int
ntpcal_ntp_to_date(struct calendar * /* jd */, uint32_t /* ntp */,
- const time_t * /* pivot */);
+ time_t /* pivot */);
extern time_t
ntpcal_date_to_time(const struct calendar * /* jd */);
=====================================
include/ntp_fp.h
=====================================
--- a/include/ntp_fp.h
+++ b/include/ntp_fp.h
@@ -160,6 +160,7 @@ extern char * mfptoa (l_fp, short);
extern char * mfptoms (l_fp, short);
extern bool hextolfp (const char *, l_fp *);
+extern void set_prettydate_pivot(time_t);
extern char * prettydate (const l_fp);
extern char * rfc3339date (const l_fp);
extern char * rfc3339time (time_t);
=====================================
include/ntp_stdlib.h
=====================================
--- a/include/ntp_stdlib.h
+++ b/include/ntp_stdlib.h
@@ -48,7 +48,7 @@ extern bool authreadkeys (const char *);
extern void authtrust (keyid_t, bool);
extern bool authusekey (keyid_t, int, const uint8_t *);
-extern int clocktime (int, int, int, int, int, int, uint32_t, uint32_t *, uint32_t *);
+extern int clocktime (int, int, int, int, int, int, time_t, uint32_t, uint32_t *, uint32_t *);
extern void init_auth (void);
extern void init_network (void);
extern void auth_prealloc_symkeys(int);
=====================================
include/timespecops.h
=====================================
--- a/include/timespecops.h
+++ b/include/timespecops.h
@@ -61,7 +61,7 @@ extern l_fp tspec_intv_to_lfp( struct timespec);
extern l_fp tspec_stamp_to_lfp( struct timespec);
extern struct timespec lfp_intv_to_tspec(l_fp);
extern struct timespec lfp_uintv_to_tspec(l_fp);
-extern struct timespec lfp_stamp_to_tspec(l_fp, const time_t *);
+extern struct timespec lfp_stamp_to_tspec(l_fp, time_t);
extern struct timespec tval_to_tspec(struct timeval);
#endif /* GUARD_TIMESPECOPS_H */
=====================================
libntp/clocktime.c
=====================================
--- a/libntp/clocktime.c
+++ b/libntp/clocktime.c
@@ -27,7 +27,7 @@
/*
* local calendar helpers
*/
-static int32_t ntp_to_year(uint32_t);
+static int32_t ntp_to_year(uint32_t, time_t);
static uint32_t year_to_ntp(int32_t);
/*
@@ -59,7 +59,8 @@ clocktime(
int minute , /* minute of hour */
int second , /* second of minute */
int tzoff , /* hours west of GMT */
- uint32_t rec_ui , /* pivot value */
+ time_t pivot , /* pivot for time unfolding */
+ uint32_t rec_ui , /* recent timestamp to get year from */
uint32_t *yearstart, /* cached start-of-year, secs from NTP epoch */
uint32_t *ts_ui ) /* effective time stamp */
{
@@ -128,7 +129,7 @@ clocktime(
* around the guess and select the entry with the minimum
* absolute difference to the receive time stamp.
*/
- y = ntp_to_year(rec_ui - (unsigned int)tmp);
+ y = ntp_to_year(rec_ui - (unsigned int)tmp, pivot);
for (idx = 0; idx < 3; idx++) {
/* -- get year start of potential solution */
ystt[idx] = year_to_ntp(y + idx - 1);
@@ -153,12 +154,13 @@ clocktime(
static int32_t
ntp_to_year(
- uint32_t ntp)
+ uint32_t ntp,
+ time_t pivot)
{
time64_t t;
ntpcal_split s;
- t = ntpcal_ntp_to_ntp(ntp, NULL);
+ t = ntpcal_ntp_to_ntp(ntp, pivot);
s = ntpcal_daysplit(t);
s = ntpcal_split_eradays(s.hi + DAY_NTP_STARTS - 1, NULL);
return s.hi + 1;
=====================================
libntp/ntp_calendar.c
=====================================
--- a/libntp/ntp_calendar.c
+++ b/libntp/ntp_calendar.c
@@ -22,37 +22,12 @@
* --------------------------------------------------------------------
*/
-static systime_func_ptr systime_func = &time;
-static inline time_t now(void);
-
static ntpcal_split
ntpcal_days_in_months(int32_t /* months */);
static int32_t
ntpcal_edate_to_yeardays(int32_t, int32_t, int32_t);
-systime_func_ptr
-ntpcal_set_timefunc(
- systime_func_ptr nfunc
- )
-{
- systime_func_ptr res;
-
- res = systime_func;
- if (NULL == nfunc)
- nfunc = &time;
- systime_func = nfunc;
-
- return res;
-}
-
-
-static inline time_t
-now(void)
-{
- return (*systime_func)(NULL);
-}
-
/*
*---------------------------------------------------------------------
* Get the build date & time
@@ -73,10 +48,10 @@ ntpcal_get_build_date(
epoch_tm = gmtime(&epoch);
if ( NULL == epoch_tm ) {
- /* bad POCH */
+ /* bad EPOCH */
return false;
}
- /* good EOPCH */
+ /* good EPOCH */
jd->year = epoch_tm->tm_year + 1900;
jd->yearday = epoch_tm->tm_yday + 1;
jd->month = epoch_tm->tm_mon + 1;
@@ -289,12 +264,12 @@ ntpcal_periodic_extend(
time64_t
ntpcal_ntp_to_time(
uint32_t ntp,
- const time_t * pivot
+ time_t pivot
)
{
time64_t res;
- settime64s(res, (pivot != NULL) ? *pivot : now());
+ settime64s(res, pivot);
settime64u(res, time64u(res)-0x80000000); /* unshift of half range */
ntp -= (uint32_t)JAN_1970; /* warp into UN*X domain */
ntp -= time64lo(res); /* cycle difference */
@@ -319,12 +294,12 @@ ntpcal_ntp_to_time(
time64_t
ntpcal_ntp_to_ntp(
uint32_t ntp,
- const time_t *pivot
+ time_t pivot
)
{
time64_t res;
- settime64s(res, (pivot) ? *pivot : now());
+ settime64s(res, pivot);
settime64u(res, time64u(res) - 0x80000000); /* unshift of half range */
settime64u(res, time64u(res) + (uint32_t)JAN_1970); /* warp into NTP domain */
@@ -887,7 +862,7 @@ int
ntpcal_ntp_to_date(
struct calendar *jd,
uint32_t ntp,
- const time_t *piv
+ const time_t piv
)
{
time64_t ntp64;
=====================================
libntp/prettydate.c
=====================================
--- a/libntp/prettydate.c
+++ b/libntp/prettydate.c
@@ -114,6 +114,13 @@ get_struct_tm(
return tm;
}
+static time_t prettypivot;
+
+void
+set_prettydate_pivot(time_t pivot) {
+ prettypivot = pivot;
+}
+
static char *
common_prettydate(
const l_fp ts
@@ -137,7 +144,7 @@ common_prettydate(
msec -= 1000u;
ntps++;
}
- sec = ntpcal_ntp_to_time(ntps, NULL);
+ sec = ntpcal_ntp_to_time(ntps, prettypivot);
tm = get_struct_tm(&sec, &tmbuf);
if (!tm) {
/*
=====================================
libntp/pymodule.c
=====================================
--- a/libntp/pymodule.c
+++ b/libntp/pymodule.c
@@ -97,7 +97,7 @@ ntpc_lfptofloat(PyObject *self, PyObject *args)
PyErr_SetString(PyExc_ValueError, "ill-formed hex date");
return NULL;
}
- tt = lfp_stamp_to_tspec(ts, NULL);
+ tt = lfp_stamp_to_tspec(ts, time(NULL));
return Py_BuildValue("d", tt.tv_sec + tt.tv_nsec * S_PER_NS);
}
=====================================
libntp/systime.c
=====================================
--- a/libntp/systime.c
+++ b/libntp/systime.c
@@ -376,7 +376,7 @@ step_systime(
fp_sys += fp_ofs;
/* unfold the new system time */
- timets = lfp_stamp_to_tspec(fp_sys, &pivot);
+ timets = lfp_stamp_to_tspec(fp_sys, pivot);
/* now set new system time */
if (settime(&timets) != 0) {
=====================================
libntp/timespecops.c
=====================================
--- a/libntp/timespecops.c
+++ b/libntp/timespecops.c
@@ -335,12 +335,12 @@ lfp_uintv_to_tspec(
/*
* absolute (timestamp) conversion. Input is time in NTP epoch, output
* is in UN*X epoch. The NTP time stamp will be expanded around the
- * pivot time *p or the current time, if p is NULL.
+ * pivot time p.
*/
struct timespec
lfp_stamp_to_tspec(
l_fp x,
- const time_t * p
+ time_t p
)
{
struct timespec out;
=====================================
ntpd/ntp_refclock.c
=====================================
--- a/ntpd/ntp_refclock.c
+++ b/ntpd/ntp_refclock.c
@@ -414,7 +414,7 @@ refclock_process_f(
* it finds only a 2-digit year in the timecode.
*/
if (!clocktime(pp->year, pp->day, pp->hour, pp->minute, pp->second, GMT,
- lfpuint(pp->lastrec), &pp->yearstart, &sec))
+ time(NULL), lfpuint(pp->lastrec), &pp->yearstart, &sec))
return false;
setlfpuint(offset, sec);
=====================================
ntpd/ntpd.c
=====================================
--- a/ntpd/ntpd.c
+++ b/ntpd/ntpd.c
@@ -564,6 +564,8 @@ ntpdmain(
exit(1);
}
+ set_prettydate_pivot(time(NULL));
+
# ifdef HAVE_WORKING_FORK
/* make sure the FDs are initialised */
pipe_fds[0] = -1;
=====================================
ntpd/refclock_nmea.c
=====================================
--- a/ntpd/refclock_nmea.c
+++ b/ntpd/refclock_nmea.c
@@ -1686,7 +1686,7 @@ unfold_day(
* cannot assume we can do this; therefore this is done
* in split representation.
*/
- rec_qw = ntpcal_ntp_to_ntp(rec_ui - SECSPERDAY/2, NULL);
+ rec_qw = ntpcal_ntp_to_ntp(rec_ui - SECSPERDAY/2, time(NULL));
rec_ds = ntpcal_daysplit(rec_qw);
rec_ds.lo = ntpcal_periodic_extend(rec_ds.lo,
ntpcal_date_to_daysec(jd),
@@ -1720,7 +1720,7 @@ unfold_century(
struct calendar rec;
int32_t baseyear;
- ntpcal_ntp_to_date(&rec, rec_ui, NULL);
+ ntpcal_ntp_to_date(&rec, rec_ui, time(NULL));
baseyear = rec.year - 20;
if (baseyear < g_gpsMinYear)
baseyear = g_gpsMinYear;
@@ -1870,7 +1870,7 @@ eval_gps_time(
}
/* - get unfold base: day of full recv time - 512 weeks */
- vi64 = ntpcal_ntp_to_ntp(lfpuint(*xrecv), NULL);
+ vi64 = ntpcal_ntp_to_ntp(lfpuint(*xrecv), time(NULL));
rs64 = ntpcal_daysplit(vi64);
rcv_sec = rs64.lo;
rcv_day = rs64.hi - 512 * 7;
=====================================
tests/common/caltime.c
=====================================
--- a/tests/common/caltime.c
+++ b/tests/common/caltime.c
@@ -7,19 +7,12 @@
time_t nowtime = 0;
-time_t timefunc(time_t *ptr) {
- if (ptr) {
- *ptr = nowtime;
- }
- return nowtime;
-}
-
-void settime(int y, int m, int d, int H, int M, int S) {
+time_t settime(int y, int m, int d, int H, int M, int S) {
time_t days = ntpcal_edate_to_eradays(y-1, m-1, d-1) + (1 - DAY_UNIX_STARTS);
time_t secs = ntpcal_etime_to_seconds(H, M, S);
- nowtime = days * SECSPERDAY + secs;
+ return days * SECSPERDAY + secs;
}
=====================================
tests/common/caltime.h
=====================================
--- a/tests/common/caltime.h
+++ b/tests/common/caltime.h
@@ -2,6 +2,5 @@
#include "ntp_calendar.h"
-time_t timefunc(time_t*);
-void settime(int y, int m, int d, int H, int M, int S);
+time_t settime(int y, int m, int d, int H, int M, int S);
const char *CalendarToString(const struct calendar *cal);
=====================================
tests/libntp/clocktime.c
=====================================
--- a/tests/libntp/clocktime.c
+++ b/tests/libntp/clocktime.c
@@ -16,13 +16,13 @@ TEST_GROUP(clocktime);
// function for getting the current system time, so the tests are not
// dependent on the actual system time.
+static time_t fixedpivot;
+
TEST_SETUP(clocktime) {
- ntpcal_set_timefunc(timefunc);
- settime(2000, 1, 1, 0, 0, 0);
+ fixedpivot = settime(2000, 1, 1, 0, 0, 0);
}
TEST_TEAR_DOWN(clocktime) {
- ntpcal_set_timefunc(NULL);
}
// ---------------------------------------------------------------------
@@ -39,7 +39,7 @@ TEST(clocktime, CurrentYear) {
uint32_t actual;
TEST_ASSERT_TRUE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
TEST_ASSERT_EQUAL(expected, actual);
TEST_ASSERT_EQUAL(yearstart, 3471292800);
}
@@ -55,7 +55,7 @@ TEST(clocktime, CurrentYearExplicit) {
uint32_t actual;
TEST_ASSERT_TRUE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
/* If this assertion fails with "Expected 3486372600 was
* 104913720" that's a 32-bit integer overflow and your compiler
* is failing to cast to int properly inside clocktime.
@@ -83,7 +83,7 @@ TEST(clocktime, CurrentYearFuzz) {
uint32_t actual;
TEST_ASSERT_TRUE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
TEST_ASSERT_EQUAL(expected, actual);
}
@@ -103,7 +103,7 @@ TEST(clocktime, TimeZoneOffset) {
uint32_t actual;
TEST_ASSERT_TRUE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
TEST_ASSERT_EQUAL(expected, actual);
}
@@ -122,7 +122,7 @@ TEST(clocktime, WrongYearStart) {
uint32_t actual;
TEST_ASSERT_TRUE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
TEST_ASSERT_EQUAL(expected, actual);
}
@@ -141,7 +141,7 @@ TEST(clocktime, PreviousYear) {
uint32_t actual;
TEST_ASSERT_TRUE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
TEST_ASSERT_EQUAL(expected, actual);
}
@@ -159,7 +159,7 @@ TEST(clocktime, NextYear) {
uint32_t actual;
TEST_ASSERT_TRUE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
TEST_ASSERT_EQUAL(expected, actual);
}
@@ -172,7 +172,7 @@ TEST(clocktime, NoReasonableConversion) {
uint32_t actual;
TEST_ASSERT_FALSE(clocktime(year, yday, hour, minute, second,
- tzoff, timestamp, &yearstart, &actual));
+ tzoff, fixedpivot, timestamp, &yearstart, &actual));
}
TEST(clocktime, AlwaysInLimit) {
@@ -198,7 +198,7 @@ TEST(clocktime, AlwaysInLimit) {
for (hour = -204; hour < 204; hour += 2) {
for (minute = -60; minute < 60; minute++) {
(void)clocktime(0, yday, hour, minute, 30, 0,
- timestamp, &yearstart, &actual);
+ fixedpivot, timestamp, &yearstart, &actual);
diff = actual - timestamp;
if (diff >= 0x80000000UL) {
diff = ~diff + 1;
=====================================
tests/libntp/prettydate.c
=====================================
--- a/tests/libntp/prettydate.c
+++ b/tests/libntp/prettydate.c
@@ -1,12 +1,16 @@
#include "config.h"
#include "ntp_stdlib.h"
+#include "ntp_fp.h"
+#include "caltime.h"
#include "unity.h"
#include "unity_fixture.h"
TEST_GROUP(prettydate);
-TEST_SETUP(prettydate) {}
+TEST_SETUP(prettydate) {
+ set_prettydate_pivot(settime(2000, 1, 1, 0, 0, 0));
+}
TEST_TEAR_DOWN(prettydate) {}
=====================================
tests/ntpd/leapsec.c
=====================================
--- a/tests/ntpd/leapsec.c
+++ b/tests/ntpd/leapsec.c
@@ -12,17 +12,16 @@
TEST_GROUP(leapsec);
+static time_t fixedpivot;
+
TEST_SETUP(leapsec) {
- ntpcal_set_timefunc(timefunc);
- settime(1970, 1, 1, 0, 0, 0);
+ fixedpivot = settime(1970, 1, 1, 0, 0, 0);
leapsec_ut_pristine();
}
TEST_TEAR_DOWN(leapsec) {
- ntpcal_set_timefunc(NULL);
}
-
static const char leap1 [] =
"#\n"
"#@ 3610569600\n"
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/1815910a431893e4e2ed1a1ee3d819a4dd45e6ea
---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/1815910a431893e4e2ed1a1ee3d819a4dd45e6ea
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20171127/d225feb5/attachment.html>
More information about the vc
mailing list