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

fix: localtime error #113

Merged
merged 6 commits into from
Aug 8, 2021
Merged

fix: localtime error #113

merged 6 commits into from
Aug 8, 2021

Conversation

fuchengjie
Copy link
Contributor

when environment is win10 and vs2019
i encountered this problem
image

And add this macro can solve it.

@fuchengjie
Copy link
Contributor Author

@sharkdp,check it pls

@sharkdp
Copy link
Owner

sharkdp commented Jul 4, 2021

I'm not sure this is the correct way to fix this problem (silencing the warning). How about applying an actual fix (switch to localtime_s)?

@fuchengjie
Copy link
Contributor Author

I will try it

@fuchengjie
Copy link
Contributor Author

@sharkdp, i use localtime_s to fix error, pls check it.

@fuchengjie fuchengjie changed the title fix :localtime error fix :add localtime_s Jul 5, 2021
@fuchengjie fuchengjie changed the title fix :add localtime_s fix :localtime error Jul 5, 2021
@fuchengjie fuchengjie changed the title fix :localtime error fix: localtime error Jul 5, 2021
dbg.h Outdated
@@ -550,7 +550,12 @@ inline bool pretty_print(std::ostream& stream, const time&) {
const auto us =
duration_cast<microseconds>(now.time_since_epoch()).count() % 1000000;
const auto hms = system_clock::to_time_t(now);
#if _MSC_VER >= 1600
const auto tm = new std::tm;
Copy link
Owner

Choose a reason for hiding this comment

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

This allocates a new tm object on the heap and never releases the memory.

dbg.h Show resolved Hide resolved
dbg.h Outdated
stream << "current time = " << std::put_time(tm, "%H:%M:%S") << '.'
<< std::setw(6) << std::setfill('0') << us;

delete tm;
Copy link
Owner

@sharkdp sharkdp Jul 8, 2021

Choose a reason for hiding this comment

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

Now you are calling delete on a tm* pointer to a static tm object inside std::localtime when the #else branch is active (leading to the CI crashes on non-windows environments). Please don't use new/delete at all here but rather simply "stack-allocate" a tm object locally.

@sharkdp sharkdp merged commit 4db6180 into sharkdp:master Aug 8, 2021
@sharkdp
Copy link
Owner

sharkdp commented Aug 8, 2021

Thank you very much!

@fuchengjie
Copy link
Contributor Author

Thank you very much!

It's my pleasure.

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.

2 participants