This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: PyModule_GetDict() claims it can never fail, but it can
Type: behavior Stage: resolved
Components: Documentation Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: amaury.forgeotdarc, berker.peksag, docs@python, matrixise, python-dev, scoder, vstinner
Priority: normal Keywords: patch

Created on 2011-09-09 16:19 by scoder, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue12946.diff berker.peksag, 2016-08-06 10:22 review
Messages (10)
msg143764 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2011-09-09 16:19
As is obvious from the code, PyModule_GetDict() can fail if being passed a non-module object, and when the (unlikely) dict creation at the end fails. The documentation of the C-API function should be fixed to reflect that, i.e. it should state that NULL is returned in the case of an error.

PyObject *
PyModule_GetDict(PyObject *m)
{
    PyObject *d;
    if (!PyModule_Check(m)) {
        PyErr_BadInternalCall();
        return NULL;
    }
    d = ((PyModuleObject *)m) -> md_dict;
    if (d == NULL)
        ((PyModuleObject *)m) -> md_dict = d = PyDict_New();
    return d;
}
msg143940 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-09-12 21:40
The path with PyDict_New() is never taken, because PyModule_New already fills md_dict.
msg143994 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2011-09-13 23:13
So, can we close this issue?
msg144007 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2011-09-14 04:06
I gave two reasons why this function can fail, and one turns out to be assumed-to-be-dead code. So, no, there are two issues now, one with the documentation, one with the code.
msg227512 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-09-25 07:33
> I gave two reasons why this function can fail, and one turns out to be assumed-to-be-dead code. 

If the call to PyDict_New() is never called, the test can be replaced with an assertion.
msg272088 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-06 10:22
I've changed the dead code to

    assert(d != NULL);

and added the following sentence to PyModule_GetDict documentation:

    If *module* is not a module object (or a subtype of a module object),
    :exc:`SystemError` is raised and *NULL* is returned.
msg272113 - (view) Author: Stéphane Wirtel (matrixise) * (Python committer) Date: 2016-08-07 07:39
you can merge the patch.
msg273098 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-19 08:51
New changeset bd2a4c138b76 by Berker Peksag in branch '3.5':
Issue #12946: Document that PyModule_GetDict can fail in some cases
https://hg.python.org/cpython/rev/bd2a4c138b76

New changeset c2e74b88947d by Berker Peksag in branch 'default':
Issue #12946: Merge from 3.5
https://hg.python.org/cpython/rev/c2e74b88947d
msg273099 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-19 08:59
New changeset 4b64a049f451 by Berker Peksag in branch 'default':
Issue #12946: Remove dead code in PyModule_GetDict
https://hg.python.org/cpython/rev/4b64a049f451
msg273100 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-19 09:00
Thanks for the report, Stefan!
History
Date User Action Args
2022-04-11 14:57:21adminsetgithub: 57155
2016-08-19 09:00:17berker.peksagsetmessages: + msg273100
2016-08-19 09:00:00berker.peksagsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-08-19 08:59:40python-devsetmessages: + msg273099
2016-08-19 08:51:51python-devsetnosy: + python-dev
messages: + msg273098
2016-08-07 07:39:06matrixisesetnosy: + matrixise
messages: + msg272113
2016-08-06 10:22:35berker.peksagsetfiles: + issue12946.diff

versions: + Python 3.6, - Python 2.6, Python 3.1, Python 2.7, Python 3.2, Python 3.3, Python 3.4
keywords: + patch
nosy: + berker.peksag

messages: + msg272088
stage: patch review
2014-09-25 07:33:17vstinnersetmessages: + msg227512
2014-09-25 07:25:47themesetversions: + Python 3.4, Python 3.5
2011-09-14 04:06:14scodersetmessages: + msg144007
2011-09-13 23:13:03vstinnersetnosy: + vstinner
messages: + msg143994
2011-09-12 21:40:03amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg143940
2011-09-09 16:19:51scodercreate