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

Check result of localtime #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mattn
Copy link

@mattn mattn commented Apr 6, 2020

On Windows, localtime(1586127053815)->strftime("%Y") always crash.

@mattn mattn changed the title Localtime null Check result of localtime Apr 6, 2020
On Windows, localtime(1586127053815)->strftime("%Y") always crash.
@smith153
Copy link
Collaborator

smith153 commented Apr 6, 2020

Interesting, good catch! There is quite a bit of more error checking that needs to be added to that file (currently on the TODO list). I will add this and hopefully some more checks soon.

@johannessen
Copy link

Another test case that apparently makes Time::Piece 1.3401 crash on Win32:

gmtime( -86400 )->strftime( '%Y-%m-%d' );

For what it’s worth, dates before 1 January 1970 are documented to fail on Win32 in Microsoft’s gmtime docs. CPAN Testers results suggest that the above correctly produces 1969-12-31 in Cygwin.

I don’t run Windows myself, but from reading this patch, it does look like it would at least avoid the crash and I think it should be merged in.

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.

3 participants