[issue12946] PyModule_GetDict() claims it can never fail, but it can
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
New submission from Stefan Behnel <scoder@users.sourceforge.net>: 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; } ---------- assignee: docs@python components: Documentation messages: 143764 nosy: docs@python, scoder priority: normal severity: normal status: open title: PyModule_GetDict() claims it can never fail, but it can type: behavior versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3 _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Amaury Forgeot d'Arc <amauryfa@gmail.com> added the comment: The path with PyDict_New() is never taken, because PyModule_New already fills md_dict. ---------- nosy: +amaury.forgeotdarc _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
STINNER Victor <victor.stinner@haypocalc.com> added the comment: So, can we close this issue? ---------- nosy: +haypo _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Stefan Behnel <scoder@users.sourceforge.net> added the comment: 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. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Changes by theme <theemathas@gmail.com>: ---------- versions: +Python 3.4, Python 3.5 _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
STINNER Victor added the comment:
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. ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Berker Peksag added the comment: 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. ---------- keywords: +patch nosy: +berker.peksag stage: -> patch review versions: +Python 3.6 -Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 3.4 Added file: http://bugs.python.org/file44030/issue12946.diff _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Stéphane Wirtel added the comment: you can merge the patch. ---------- nosy: +matrixise _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Roundup Robot added the comment: 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 ---------- nosy: +python-dev _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Roundup Robot added the comment: New changeset 4b64a049f451 by Berker Peksag in branch 'default': Issue #12946: Remove dead code in PyModule_GetDict https://hg.python.org/cpython/rev/4b64a049f451 ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Changes by Berker Peksag <berker.peksag@gmail.com>: ---------- resolution: -> fixed stage: patch review -> resolved status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
![](https://secure.gravatar.com/avatar/fa0f7819f1825f596b384c19aa7dcf33.jpg?s=120&d=mm&r=g)
Berker Peksag added the comment: Thanks for the report, Stefan! ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue12946> _______________________________________
participants (7)
-
Amaury Forgeot d'Arc
-
Berker Peksag
-
Roundup Robot
-
Stefan Behnel
-
STINNER Victor
-
Stéphane Wirtel
-
theme