Skip to content
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

mktime64 test fixes, and optimizations #14

Merged
merged 26 commits into from
Apr 19, 2017

Conversation

rurban
Copy link

@rurban rurban commented May 29, 2016

merge and fix rdiez' improvements.
improve the Makefile, detect Linux and Darwin, and add the needed HAS_TM defines.
support make DEBUG=1 and USE_TM64=1
add tap.h declarations
replace %lld printf with %PRId64 from inttypes.h
Fix fragile t/mktime64.t
use an hour which does not overflow to the next or prev day.
test all kind of years, esp. leap years with out of bound dates

various minor optimizations

rdiez and others added 26 commits May 28, 2016 15:43
…he C99 standard says.

Two leap seconds in the same minute are not allowed (the C90 range 0..61 was a defect).
…es not actually guarantee that the 'true' value of a boolean expression is exactly 1.
preprocessed with -E
use the right format printf specifier, long long or just long
fix t/asctime64.t.c, use the result of asctime_r_ret2
add -DHAS_TM_TM_GMTOFF -DHAS_TM_TM_ZONE to linux, using _DEFAULT_SOURCE
tap: cast year to year_t (int or long)
time64.h: dont want to include inttypes.h, so hack our own PRId64 define
use an hour which does not overflow to the next or prev day.
test all kind of years, esp. leap years with out of bound dates.

add simplier IS_LEAP_ABS test for absolute years, only > 0.
optimize seconds_between_years adjustment.
on older glibc -DHAS_TM_TM_GMTOFF -DHAS_TM_TM_ZONE is wrong
@jakesylvestre
Copy link

+1 on merge.

Copy link
Collaborator

@schwern schwern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a .i file for?

t/mktime64.t.c Outdated
@@ -50,6 +49,12 @@ int main(void) {
date.tm_mday = 37;
date.tm_wday = 9; /* deliberately wrong week day */
date.tm_yday = 487; /* and wrong year day */
#ifdef HAS_TM_TM_GMTOFF
/*date.tm_gmtoff = 0;*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be uncommented?

t/tap.h Outdated
@@ -1,6 +1,12 @@
#include <stdarg.h>
#include "time64.h"

#ifdef USE_TM64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need to define a year_t in the library itself.

@@ -87,8 +86,7 @@ int is_str(const char* have, const char* want, const char *message, ...) {
do_test( test, message, args );

if( !test ) {
diag("have: %s", have);
diag("want: %s", want);
diag("have: %s, want: %s", have, want);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have and want are supposed to be on two lines, the two values are lined up to more easily see the difference.

t/tap.c Outdated
@@ -106,8 +104,7 @@ int is_Int64(const Int64 have, const Int64 want, const char *name, ...) {
do_test( test, name, args );

if( !test ) {
diag("have: %"PRId64, have);
diag("want: %"PRId64, want);
diag("have: %"PRId64" want: %"PRId64" diff: %"PRId64, have, want, have-want);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Makefile Outdated
@@ -20,7 +20,8 @@ ifeq ($(UNAME_S),Linux)
# (instead of __tm_gmtoff and __tm_zone) in struct tm,
# and _POSIX_SOURCE (there are alternatives) for tzset().
# _BSD_SOURCE is deprecated, use _DEFAULT_SOURCE instead
CFLAGS += -D_DEFAULT_SOURCE -D_POSIX_SOURCE -DHAS_TM_TM_GMTOFF -DHAS_TM_TM_ZONE
CFLAGS += -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_POSIX_SOURCE
#CFLAGS += -D_DEFAULT_SOURCE -D_POSIX_SOURCE -DHAS_TM_TM_GMTOFF -DHAS_TM_TM_ZONE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminding myself to remove this commented out line.

@schwern schwern merged commit f50a90b into evalEmpire:master Apr 19, 2017
This was referenced Apr 19, 2017
@schwern
Copy link
Collaborator

schwern commented Apr 19, 2017

Thanks a lot for doing all that work! Sorry it took so long to review and merge.

@rurban
Copy link
Author

rurban commented Apr 19, 2017

What's a .i file for?

It's the naming convention for cc -E preprocessed .c files. You inspect macro expansions and included headers.

@rurban
Copy link
Author

rurban commented Apr 19, 2017

Welcome back! We missed you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants