Hello,
Just like [i]date()
, getdate()
without the $timestamp
argument uses the current timestamp.
The code for [i]date()
(https://github.com/php/php-src/blob/master/ext/date/php_date.c#L1276) only makes a system call to time(NULL)
when the $timestamp
argument is missing. In contrast getdate()
(https://github.com/php/php-src/blob/master/ext/date/php_date.c#L1828) always performs a system call to time(NULL)
even if $timestamp
is supplied by the user.
The code for getdate()
should mimic what is done for date()
to avoid unnecessary system call overhead, hence only call time(NULL)
when !ZEND_NUM_ARGS()
.
Can someone please make the appropriate change? Please apologize, but I simply review code and do not have a proper C development setup that allows me to supply patches or create pull requests.
Thanks,
Ben
--
Benjamin Coutu
Just like
[i]date()
,getdate()
without the$timestamp
argument uses the current timestamp.The code for
[i]date()
(https://github.com/php/php-src/blob/master/ext/date/php_date.c#L1276) only makes a system call totime(NULL)
when the$timestamp
argument is missing. In contrastgetdate()
(https://github.com/php/php-src/blob/master/ext/date/php_date.c#L1828) always performs a system call totime(NULL)
even if$timestamp
is supplied by the user.The code for
getdate()
should mimic what is done fordate()
to avoid unnecessary system call overhead, hence only calltime(NULL)
when!ZEND_NUM_ARGS()
.
ACK. I wonder, though, whether it is safe to convert a time_t
(which
is supposed to be returned by time()
) to zend_long
. POSIX 6 claims
that time_t
shall be an integer or real-floating type[1]; POSIX 7 is
more specific ("time_t shall be an integer type")[2], but obviously
there are neither guarantees regarding signess nor size, and it doesn't
seem that we have respective checks in place[3].
[1]
http://pubs.opengroup.org/onlinepubs/009695299/basedefs/sys/types.h.html
[2]
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
[3] https://github.com/php/php-src/blob/master/configure.ac
--
Christoph M. Becker