Skip to content

loader: check for null pointer in getArray() and getString() #381

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

Closed
wants to merge 1 commit into from

Conversation

jedisct1
Copy link

When NULL pointers are returned, the loader's module getArray() and getString() return arbitrary data sitting at offset 0.

Even though null shouldn't be returned for error handling, this can happen in real code.

Throwing an exception instead prevents undefined behaviors.

@dcodeIO
Copy link
Member

dcodeIO commented Dec 19, 2018

I think that instead of throwing, these functions should just return null because the underlying string or array reference type might be nullable. Wdyt?

@jedisct1
Copy link
Author

Calling getString() on a null pointer, no matter what the underlying type is, should throw an exception IMHO. Can't think of use case where that wouldn't be due to a bug.

@dcodeIO
Copy link
Member

dcodeIO commented May 16, 2020

This has been somewhat implemented by means of an id check meanwhile. In general I think that we should strive to avoid potentially redundant checks like the one proposed here so we don't find us in a situation where these add up, and instead document the behavior more thoroughly, so users are aware that if they need to check, they must check.

Closing this PR as part of 2020 vacuum because the loader has changed substantially meanwhile, and creating a new PR documenting the behavior.

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