gh-106307: Fix PyMapping_GetOptionalItemString() - #108797
Conversation
Sorry, something went wrong.
The resulting pointer was not set to NULL if the creation of a temporary string object was failed. The tests were also missed due to oversight.
vstinner
left a comment
There was a problem hiding this comment.
LGTM. I just propose adding a comment on the new macro.
Does Valgrind or similar tool detect the usage of uninitialized memory?
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
The _testcapi now intializes the variable used in the test:
static PyObject *
mapping_getoptionalitemstring(PyObject *self, PyObject *args)
{
PyObject *obj, *value = UNINITIALIZED_PTR;
Sorry, something went wrong.
|
@erlend-aasland: Idea for the devguide: We should suggest to always initialized the &value parameter to NULL in case of errors. I just avoid any risk of using an uninitialized variable by misusing the C API. What do you think? |
Sorry, something went wrong.
I have a PR for this, and you already approved it twice 😃 python/devguide#1128 |
Sorry, something went wrong.
|
There are exceptions, like _PyEval_SliceIndex(), where the output parameter should not be set in some cases. |
Sorry, something went wrong.
|
Exceptions are fine; guidelines are for the general case, not the special case. |
Sorry, something went wrong.
Alright, I should approve it 3 or 4 times 👍👍👍👍 |
Sorry, something went wrong.
The resulting pointer was not set to NULL if the creation of a temporary string object was failed.
The tests were also missed due to oversight.