Repository navigation
gh-151126: Check strdup result in readline on_completion - #156299
KowalskiThomas wants to merge 1 commit into
Conversation
Add a NULL check and an explicit PyErr_NoMemory() before goto error.
| @@ -1272,6 +1272,10 @@ on_completion(const char *text, int state) | |||
| goto error; | |||
| result = strdup(PyBytes_AS_STRING(encoded)); | |||
There was a problem hiding this comment.
I think this should actually change to _PyMem_Strdup while we're here
There was a problem hiding this comment.
Will do depending on whether we just close the PR or not!
| result = strdup(PyBytes_AS_STRING(encoded)); | ||
| Py_DECREF(encoded); | ||
| if (result == NULL) { | ||
| PyErr_NoMemory(); |
There was a problem hiding this comment.
I don't think this works right as the goto err will clear the memory error set
There was a problem hiding this comment.
🤔 You're right, indeed.
Looking a bit more at it, I'm starting to wonder if the lack of checking strdup result is an oversight or on purpose, since the function ignores all errors that can happen and doesn't seem to ever return / raise them.
So I'm not quite sure what I should do here. It seems like actually bubbling the exception all the way up to input would require "setting the exception" somewhere, so that the actual "reading loop" can detect it and abort/actually raise the exception.
Alternatively, it might make sense to just explicitly ignore the result of strdup (since at this point it's likely the user impact is none or near none), and if strdup it's also likely things will blow up not much later...
|
Seems like this really is an unlikely edge case and there's no straightforward fix to it (see this comment) so I will just close the PR. |
2 participants
Relates to #151126.
PyErr_NoMemory()call after failing memory allocation #151126