-
Notifications
You must be signed in to change notification settings - Fork 60
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 if the data passed to the frame model is not a nullptr #578
Conversation
include/podio/Frame.h
Outdated
m_parameters(std::move(m_data->getParameters())) { | ||
m_mapMtx(std::make_unique<std::mutex>()), m_dataMtx(std::make_unique<std::mutex>()) { | ||
if (!data) { | ||
throw std::invalid_argument("Building a Frame failed; if you are reading from a file it may be corrupted"); |
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.
throw std::invalid_argument("Building a Frame failed; if you are reading from a file it may be corrupted"); | |
throw std::runtime_error("Building a Frame failed; if you are reading from a file it may be corrupted"); |
Or this, initially I was going to have a message about FrameModel but no one will know what that is so it would be a bit useless.
In principle you could also be reading past the available number of entries, as I originally envisioned the user to check whether the Lines 149 to 155 in b94cc63
I would then make the exception message mainly about receiving a |
825f825
to
3bff8dc
Compare
Changed. I prefer a more verbose error message because it's a public one and easy to get either by reading more entries than there are or by unhealthy files (I don't remember exactly how but I was able to create files that reach until there but then the data is |
Not entirely sure why the |
BEGINRELEASENOTES
ENDRELEASENOTES
It looks like a good idea to check if the pointer is null or not regardless. This makes opening corrupted or wrong files throw
instead of crashing.