Skip to content

BUG: Add missing error handling to _convert_from_list - #15291

Merged
mattip merged 1 commit into
numpy:masterfrom
eric-wieser:cleanup-descriptor-error-handling
Jan 10, 2020
Merged

mattip merged 1 commit into
numpy:masterfrom
eric-wieser:cleanup-descriptor-error-handling

Conversation

@eric-wieser

Copy link
Copy Markdown
Member

Previously this function would return null without setting an exception.
Additionally, there were many places it could segfault if an exception occurred.

Previously this function would return null without setting an exception.
Additionally, there were many places it could segfault if an exception occurred.
@mattip

mattip commented Jan 9, 2020

Copy link
Copy Markdown
Member

Is there a test we could add that hits some of these code paths?

@seberg

seberg commented Jan 10, 2020

Copy link
Copy Markdown
Member

I do not really think so, none of these should ever fail, unless in low memory situations or so?

}
/* End ignore code.*/
if (n == 0) {
PyErr_SetString(PyExc_ValueError, "Expected at least one field name");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is probably the only testable path, and I'm not sure it's worth the effort.

@mattip
mattip merged commit 624d34c into numpy:master Jan 10, 2020
@mattip

mattip commented Jan 10, 2020

Copy link
Copy Markdown
Member

By visual inspection this seems right. We may want to add a custom allocator that can be told to fail via an external parameter (an environment variable or so) that would check all these untested code paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants