-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: add null pointer check with std::localtime #2846
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
Conversation
include/pybind11/chrono.h
Outdated
// (https://en.cppreference.com/w/cpp/chrono/c/localtime) | ||
std::tm *localtime_ptr = std::localtime(&tt); | ||
if (!localtime_ptr) | ||
pybind11_fail("pybind11::chrono: Unable to represent system_clock in local time"); |
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.
pybind11_fail
is meant for internal errors, so this is not correct. Maybe throw py::cast_error
? Maybe have a look at what other casters do, please.
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.
How about 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.
If the CI wants to turn green, this seems better, indeed. Thanks! :-)
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.
I am not sure what broke in test_recarray
in test_numpy_dtypes.py on CentOS 7. I will try debugging it, now that it has failed twice.
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.
Very weird that it would be related, though
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.
It's not, it's started breaking everywhere.
bb1628b
to
438c6dd
Compare
Please don't remove the changelog part of the template, we use that to generate the changelog. Otherwise, fine! |
Description
Related conversation https://gitter.im/pybind/Lobby?at=601be2a61ed88c58d81d719a
Suggested changelog entry:
* Add null pointer check with ``std::localtime``.