-
Notifications
You must be signed in to change notification settings - Fork 8k
Refactor Internal Time Retrieval Handling for Improved Consistency and Resolution #19202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
cdebcf6
5146bcb
9bcfa57
d7ffbbf
313143d
abf0e46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/* | ||
+----------------------------------------------------------------------+ | ||
| Copyright (c) The PHP Group | | ||
+----------------------------------------------------------------------+ | ||
| This source file is subject to version 3.01 of the PHP license, | | ||
| that is bundled with this package in the file LICENSE, and is | | ||
| available through the world-wide-web at the following url: | | ||
| https://www.php.net/license/3_01.txt | | ||
| If you did not receive a copy of the PHP license and are unable to | | ||
| obtain it through the world-wide-web, please send a note to | | ||
| [email protected] so we can mail you a copy immediately. | | ||
+----------------------------------------------------------------------+ | ||
| Author: Marc Bennewitz <[email protected]> | | ||
+----------------------------------------------------------------------+ | ||
*/ | ||
|
||
#include "zend_time.h" | ||
|
||
ZEND_API void zend_realtime_spec(struct timespec *ts) { | ||
#if defined(HAVE_CLOCK_GETTIME) | ||
|
||
clock_gettime(CLOCK_REALTIME, ts); | ||
|
||
#elif defined(HAVE_TIMESPEC_GET) | ||
|
||
timespec_get(ts, TIME_UTC); | ||
|
||
#elif defined(HAVE_GETTIMEOFDAY) | ||
|
||
struct timeval tv; | ||
gettimeofday(&tv, NULL); | ||
zend_time_val2spec(tv, ts); | ||
|
||
#else | ||
|
||
ts->tv_sec = zend_realtime_get(); | ||
ts->tv_nsec = 0; | ||
|
||
#endif | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please make all the macros that could be (inline) functions a proper function? I think that should be all of them. Please also properly “namespace” all the symbols by prefixing them with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
👍
For the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes. While it might sound a little awkward, I find it valuable to easily locate where the function is declared. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
/* | ||
+----------------------------------------------------------------------+ | ||
| Copyright (c) The PHP Group | | ||
+----------------------------------------------------------------------+ | ||
| This source file is subject to version 3.01 of the PHP license, | | ||
| that is bundled with this package in the file LICENSE, and is | | ||
| available through the world-wide-web at the following url: | | ||
| https://www.php.net/license/3_01.txt | | ||
| If you did not receive a copy of the PHP license and are unable to | | ||
| obtain it through the world-wide-web, please send a note to | | ||
| [email protected] so we can mail you a copy immediately. | | ||
+----------------------------------------------------------------------+ | ||
| Author: Marc Bennewitz <[email protected]> | | ||
+----------------------------------------------------------------------+ | ||
*/ | ||
|
||
#ifndef ZEND_TIME_H | ||
#define ZEND_TIME_H | ||
|
||
#include "zend_portability.h" | ||
|
||
#ifdef PHP_WIN32 | ||
# include "win32/time.h" | ||
#endif | ||
#ifdef HAVE_SYS_TIME_H | ||
# include <sys/time.h> | ||
#endif | ||
#include <time.h> | ||
|
||
#include "zend_hrtime.h" | ||
|
||
#ifndef PHP_WIN32 | ||
# define tv_sec_t time_t | ||
# define tv_usec_t suseconds_t | ||
#else | ||
# define tv_sec_t long | ||
# define tv_usec_t long | ||
#endif | ||
|
||
#define ZEND_MILLI_IN_SEC 1000U | ||
#define ZEND_MICRO_IN_SEC 1000000U | ||
|
||
/* Helper macro to assign seconds to timeval */ | ||
#define zend_time_sec2val(s, tv) \ | ||
(tv).tv_sec = (tv_sec_t) (s); \ | ||
(tv).tv_usec = 0; | ||
|
||
/* Helper macro to assign microseconds to timeval */ | ||
#define zend_time_usec2val(usec, tv) \ | ||
(tv).tv_sec = (tv_sec_t) ((usec) / ZEND_MICRO_IN_SEC); \ | ||
(tv).tv_usec = (tv_usec_t) ((usec) % ZEND_MICRO_IN_SEC); | ||
|
||
/* Helper macro to assign double (seconds) to timeval */ | ||
#define zend_time_dbl2val(d, tv) \ | ||
(tv).tv_sec = (tv_sec_t) (d); \ | ||
(tv).tv_usec = (tv_usec_t) (((d) - (tv).tv_sec) * ZEND_MICRO_IN_SEC); | ||
|
||
/* Helper macro to copy timeval to timespec */ | ||
#define zend_time_val2spec(tv, ts) \ | ||
(ts).tv_sec = (time_t) (tv).tv_sec; \ | ||
(ts).tv_nsec = (long) ((tv).tv_usec * 1000); | ||
|
||
BEGIN_EXTERN_C() | ||
|
||
/* Helper macro to get current timestamp in seconds */ | ||
#define zend_realtime_get() time(NULL) | ||
|
||
/* wrapper around clock_gettime/timestamp_get/gettimeofday/time */ | ||
ZEND_API void zend_realtime_spec(struct timespec *ts); | ||
|
||
/* Monotonic time in nanoseconds with a fallback to real/wall-time | ||
if no monotonic timer is available */ | ||
#if ZEND_HRTIME_AVAILABLE | ||
# define zend_monotime_fallback() (uint64_t)zend_hrtime() | ||
#else | ||
ZEND_API zend_always_inline uint64_t zend_monotime_fallback(void) { | ||
struct timespec ts; | ||
zend_realtime_spec(&ts); | ||
return ((uint64_t) ts.tv_sec * ZEND_NANO_IN_SEC) + ts.tv_nsec; | ||
} | ||
#endif | ||
|
||
END_EXTERN_C() | ||
|
||
#endif // ZEND_TIME_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,13 +23,9 @@ | |
#include "zend_attributes.h" | ||
#include "zend_interfaces.h" | ||
#include "zend_exceptions.h" | ||
#include "zend_time.h" | ||
#include "lib/timelib.h" | ||
#include "lib/timelib_private.h" | ||
#ifndef PHP_WIN32 | ||
#include <time.h> | ||
#else | ||
#include "win32/time.h" | ||
#endif | ||
|
||
#ifdef PHP_WIN32 | ||
static __inline __int64 php_date_llabs( __int64 i ) { return i >= 0? i: -i; } | ||
|
@@ -53,22 +49,6 @@ static inline long long php_date_llabs( long long i ) { return i >= 0 ? i : -i; | |
#define DATE_A64I(i, s) i = strtoll(s, NULL, 10) | ||
#endif | ||
|
||
PHPAPI time_t php_time(void) | ||
{ | ||
#ifdef HAVE_GETTIMEOFDAY | ||
struct timeval tm; | ||
|
||
if (UNEXPECTED(gettimeofday(&tm, NULL) != SUCCESS)) { | ||
/* fallback, can't reasonably happen */ | ||
return time(NULL); | ||
} | ||
|
||
return tm.tv_sec; | ||
#else | ||
return time(NULL); | ||
#endif | ||
} | ||
|
||
/* | ||
* RFC822, Section 5.1: http://www.ietf.org/rfc/rfc822.txt | ||
* date-time = [ day "," ] date time ; dd mm yy hh:mm:ss zzz | ||
|
@@ -867,7 +847,7 @@ static void php_date(INTERNAL_FUNCTION_PARAMETERS, bool localtime) | |
ZEND_PARSE_PARAMETERS_END(); | ||
|
||
if (ts_is_null) { | ||
ts = php_time(); | ||
ts = zend_realtime_get(); | ||
} | ||
|
||
RETURN_STR(php_format_date(ZSTR_VAL(format), ZSTR_LEN(format), ts, localtime)); | ||
|
@@ -1031,7 +1011,7 @@ PHP_FUNCTION(idate) | |
} | ||
|
||
if (ts_is_null) { | ||
ts = php_time(); | ||
ts = zend_realtime_get(); | ||
} | ||
|
||
ret = php_idate(ZSTR_VAL(format)[0], ts, 0); | ||
|
@@ -1111,7 +1091,7 @@ PHP_FUNCTION(strtotime) | |
now->tz_info = tzi; | ||
now->zone_type = TIMELIB_ZONETYPE_ID; | ||
timelib_unixtime2local(now, | ||
!preset_ts_is_null ? (timelib_sll) preset_ts : (timelib_sll) php_time()); | ||
!preset_ts_is_null ? (timelib_sll) preset_ts : (timelib_sll) zend_realtime_get()); | ||
|
||
t = timelib_strtotime(ZSTR_VAL(times), ZSTR_LEN(times), &error, | ||
DATE_TIMEZONEDB, php_date_parse_tzfile_wrapper); | ||
|
@@ -1162,15 +1142,15 @@ PHPAPI void php_mktime(INTERNAL_FUNCTION_PARAMETERS, bool gmt) | |
/* Initialize structure with current time */ | ||
now = timelib_time_ctor(); | ||
if (gmt) { | ||
timelib_unixtime2gmt(now, (timelib_sll) php_time()); | ||
timelib_unixtime2gmt(now, (timelib_sll) zend_realtime_get()); | ||
} else { | ||
tzi = get_timezone_info(); | ||
if (!tzi) { | ||
return; | ||
} | ||
now->tz_info = tzi; | ||
now->zone_type = TIMELIB_ZONETYPE_ID; | ||
timelib_unixtime2local(now, (timelib_sll) php_time()); | ||
timelib_unixtime2local(now, (timelib_sll) zend_realtime_get()); | ||
} | ||
|
||
now->h = hou; | ||
|
@@ -1280,7 +1260,7 @@ PHPAPI void php_strftime(INTERNAL_FUNCTION_PARAMETERS, bool gmt) | |
} | ||
|
||
if (timestamp_is_null) { | ||
timestamp = (zend_long) php_time(); | ||
timestamp = (zend_long) zend_realtime_get(); | ||
} | ||
|
||
ts = timelib_time_ctor(); | ||
|
@@ -1376,7 +1356,7 @@ PHP_FUNCTION(time) | |
{ | ||
ZEND_PARSE_PARAMETERS_NONE(); | ||
|
||
RETURN_LONG((zend_long)php_time()); | ||
RETURN_LONG((zend_long) zend_realtime_get()); | ||
} | ||
/* }}} */ | ||
|
||
|
@@ -1396,7 +1376,7 @@ PHP_FUNCTION(localtime) | |
ZEND_PARSE_PARAMETERS_END(); | ||
|
||
if (timestamp_is_null) { | ||
timestamp = (zend_long) php_time(); | ||
timestamp = (zend_long) zend_realtime_get(); | ||
} | ||
|
||
tzi = get_timezone_info(); | ||
|
@@ -1451,7 +1431,7 @@ PHP_FUNCTION(getdate) | |
ZEND_PARSE_PARAMETERS_END(); | ||
|
||
if (timestamp_is_null) { | ||
timestamp = (zend_long) php_time(); | ||
timestamp = (zend_long) zend_realtime_get(); | ||
} | ||
|
||
tzi = get_timezone_info(); | ||
|
@@ -2353,16 +2333,11 @@ static void php_date_set_time_fraction(timelib_time *time, int microsecond) | |
|
||
static void php_date_get_current_time_with_fraction(time_t *sec, suseconds_t *usec) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one can probably be removed entirely now. |
||
{ | ||
#ifdef HAVE_GETTIMEOFDAY | ||
struct timeval tp = {0}; /* For setting microsecond */ | ||
struct timespec ts; | ||
|
||
gettimeofday(&tp, NULL); | ||
*sec = tp.tv_sec; | ||
*usec = tp.tv_usec; | ||
#else | ||
*sec = time(NULL); | ||
*usec = 0; | ||
#endif | ||
zend_realtime_spec(&ts); | ||
*sec = ts.tv_sec; | ||
*usec = ts.tv_nsec / 1000; | ||
} | ||
|
||
PHPAPI bool php_date_initialize(php_date_obj *dateobj, const char *time_str, size_t time_str_len, const char *format, zval *timezone_object, int flags) /* {{{ */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,7 +129,8 @@ ZEND_END_MODULE_GLOBALS(date) | |
|
||
#define DATEG(v) ZEND_MODULE_GLOBALS_ACCESSOR(date, v) | ||
|
||
PHPAPI time_t php_time(void); | ||
/* DEPRECATED, use zend_realtime_get() instead */ | ||
#define php_time() zend_realtime_get() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please leave this as a (inline) function and add |
||
|
||
/* Backwards compatibility wrapper */ | ||
PHPAPI zend_long php_parse_date(const char *string, zend_long *now); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these all infallible at runtime? Complexity-wise this could also be an inline function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of these functions can return an error in case of using wrong/unsupported arguments.
On the same time here only the most basic functionality is used and in the previous implementation error returns where not checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but I don't think this is a reason that this should not be improved. Though it certainly raises the question if a broken clock is even recoverable.
Perhaps add a
(void)
in front to make it explicit that we don't want to handle the errors?