-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
PYTHON-3959 - NULL Initialize PyObjects #1859
Conversation
There is an existing patch(es) for this commit SHA: Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'. |
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.
LGTM
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.
Actually, I see there are some non NULL-initialized PyObjects. I think we should target all the PyObject* instantiations as I'm assuming this has only targeted the ones that leverage PyErr
, the original scope of the PR. Looking for suggestions from others.
The jira ticket, PYTHON-3959, mentions initializing vars for PyErr_Fetch. Should this PR be updating those? |
PYTHON-3959 mentions initializing |
So are we going to update the PyErr_Fetch calls? |
Yes! Sorry, In my first pass I misread which PyObject variables were called by which PyErr_ functions. Fixed. |
I know this is only changing NULLS but just to be safe we should wait until 4.9 is released to merge. |
I'll defer to Jib for the rest. |
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.
Got'em, thanks! C is a pleasure to work with 👼 |
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.
LGTM!!!
Let's hold off on merging this until we address https://jira.mongodb.org/browse/PYTHON-4636 |
NULL initialize PyObjects before calling PyErr_*
Initialize all PyObjects, not just those that included in calls to PyErr_*.
Missed a few, thanks Jib!
20e1049
to
f252a7c
Compare
NULL initialize PyObjects before calling PyErr_*