Skip to content

bpo-32381: Rewrite PyErr_ProgramText()#23700

Merged
vstinner merged 2 commits into
python:masterfrom
vstinner:pyerr_programtext
Dec 8, 2020
Merged

bpo-32381: Rewrite PyErr_ProgramText()#23700
vstinner merged 2 commits into
python:masterfrom
vstinner:pyerr_programtext

Conversation

@vstinner

@vstinner vstinner commented Dec 8, 2020

Copy link
Copy Markdown
Member

PyErr_ProgramText() now calls PyErr_ProgramTextObject().

https://bugs.python.org/issue32381

PyErr_ProgramText() now calls PyErr_ProgramTextObject().
Comment thread Python/errors.c

PyObject *filename_obj = PyUnicode_DecodeFSDefault(filename);
if (filename_obj == NULL) {
return NULL;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure that the contract of this function allows raising an exception? It seems that PyErr_ProgramTextObject takes care to clear the exception, as does err_programtext.

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.

I don't know. In case of doubt, I added a PyErr_Clear() call.

My first intent was to deprecate the function. IMO it was exposed by mistake in the stable ABI. But since PyErr_ProgramTextObject() is still needed, I prefer to rewrite PyErr_ProgramText() with PyErr_ProgramTextObject().

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks. Better safe than sorry :)

@vstinner vstinner merged commit 815506d into python:master Dec 8, 2020
@vstinner vstinner deleted the pyerr_programtext branch December 8, 2020 22:51
@vstinner

vstinner commented Dec 8, 2020

Copy link
Copy Markdown
Member Author

Thanks for the review @izbyshev.

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
PyErr_ProgramText() now calls PyErr_ProgramTextObject().
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.

4 participants