
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2016-04-27 05:36 byberker.peksag, last changed2022-04-11 14:58 byadmin.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| csv.diff | berker.peksag,2016-04-27 05:36 | review | ||
| addobject_doc.diff | berker.peksag,2016-04-27 09:52 | review | ||
| issue26868_v2.diff | berker.peksag,2016-06-10 05:37 | review | ||
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 15725 | merged | brandtbucher,2019-09-07 15:52 | |
| PR 16037 | merged | miss-islington,2019-09-12 12:11 | |
| PR 16038 | merged | miss-islington,2019-09-12 12:11 | |
| Messages (12) | |||
|---|---|---|---|
| msg264350 -(view) | Author: Berker Peksag (berker.peksag)*![]() | Date: 2016-04-27 05:36 | |
This is probably harmless, butModules/_csv.c has the following code: Py_INCREF(&Dialect_Type); if (PyModule_AddObject(module, "Dialect", (PyObject *)&Dialect_Type)) return NULL;However, PyModule_AddObject returns only -1 and 0. It also doesn't decref Dialect_Type if it returns -1 so I guess more correct code should be: Py_INCREF(&Dialect_Type); if (PyModule_AddObject(module, "Dialect", (PyObject *)&Dialect_Type) == -1) { Py_DECREF(&Dialect_Type); return NULL; }The same pattern can be found in a few more modules. | |||
| msg264358 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-04-27 07:19 | |
Testing returned value of PyModule_AddObject() is correct. This is a matter of style what to use: `if (...)`, `if (... == -1)` or `if (... < 0)`.But the problem with a leak is more general. I have opened a discussion on Python-Dev:http://comments.gmane.org/gmane.comp.python.devel/157545 . | |||
| msg264367 -(view) | Author: Berker Peksag (berker.peksag)*![]() | Date: 2016-04-27 09:52 | |
Thanks for the write-up, Serhiy.It looks like "... == -1" is more popular in the codebase (for PyModule_AddObject, "... < 0" is the most popular style).Here is a patch to document the current behavior of PyModule_AddObject. | |||
| msg264375 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-04-27 12:05 | |
Added comments on Rietveld. | |||
| msg264388 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-04-27 18:09 | |
As forModules/_csv.c, I think it is better to change the behavior of PyModule_AddObject() (issue26871). This will fix similar issues in all modules. | |||
| msg268088 -(view) | Author: Berker Peksag (berker.peksag)*![]() | Date: 2016-06-10 05:37 | |
Thanks for the review Serhiy. Here is an updated patch. | |||
| msg276977 -(view) | Author: Berker Peksag (berker.peksag)*![]() | Date: 2016-09-19 13:29 | |
Serhiy, do you have further comments about issue26868_v2.diff? | |||
| msg330940 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2018-12-03 11:53 | |
issue26868_v2.diff LGTM. | |||
| msg351260 -(view) | Author: Brandt Bucher (brandtbucher)*![]() | Date: 2019-09-06 15:57 | |
It looks like the idiom of calling PyModule_AddObject without Py_DECREF'ing on the error condition (or even checking for it at all) has spread quite a bit since this first reported. I'm preparing a PR to fix the other call sites. | |||
| msg352135 -(view) | Author: Stéphane Wirtel (matrixise)*![]() | Date: 2019-09-12 12:11 | |
New changeset224b8aaa7e8f67f748e8b7b6a4a77a25f6554651 by Stéphane Wirtel (Brandt Bucher) in branch 'master':bpo-26868: Fix example usage of PyModule_AddObject. (#15725)https://github.com/python/cpython/commit/224b8aaa7e8f67f748e8b7b6a4a77a25f6554651 | |||
| msg352140 -(view) | Author: miss-islington (miss-islington) | Date: 2019-09-12 12:26 | |
New changeset535863e3f599a6ad829204d83f144c91e44de443 by Miss Islington (bot) in branch '3.8':bpo-26868: Fix example usage of PyModule_AddObject. (GH-15725)https://github.com/python/cpython/commit/535863e3f599a6ad829204d83f144c91e44de443 | |||
| msg352141 -(view) | Author: miss-islington (miss-islington) | Date: 2019-09-12 12:29 | |
New changesetc8d1338441114fbc504625bc66607e7996018a5d by Miss Islington (bot) in branch '3.7':bpo-26868: Fix example usage of PyModule_AddObject. (GH-15725)https://github.com/python/cpython/commit/c8d1338441114fbc504625bc66607e7996018a5d | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:30 | admin | set | github: 71055 |
| 2019-09-12 12:29:12 | miss-islington | set | messages: +msg352141 |
| 2019-09-12 12:26:49 | miss-islington | set | nosy: +miss-islington messages: +msg352140 |
| 2019-09-12 12:11:39 | miss-islington | set | pull_requests: +pull_request15660 |
| 2019-09-12 12:11:32 | miss-islington | set | pull_requests: +pull_request15659 |
| 2019-09-12 12:11:23 | matrixise | set | nosy: +matrixise messages: +msg352135 |
| 2019-09-07 23:54:25 | malin | set | nosy: +malin |
| 2019-09-07 15:52:11 | brandtbucher | set | pull_requests: +pull_request15380 |
| 2019-09-06 15:57:27 | brandtbucher | set | nosy: +brandtbucher messages: +msg351260 |
| 2019-04-28 22:41:04 | nirs | set | nosy: +nirs |
| 2018-12-03 11:53:26 | serhiy.storchaka | set | versions: + Python 3.8, - Python 3.5 |
| 2018-12-03 11:53:16 | serhiy.storchaka | set | messages: +msg330940 |
| 2016-09-19 13:29:00 | berker.peksag | set | messages: +msg276977 versions: + Python 3.7 |
| 2016-06-10 05:37:40 | berker.peksag | set | files: +issue26868_v2.diff messages: +msg268088 |
| 2016-04-28 15:31:14 | berker.peksag | set | title: Incorrect check for return value of PyModule_AddObject in _csv.c -> Document PyModule_AddObject's behavior on error |
| 2016-04-27 18:09:03 | serhiy.storchaka | set | dependencies: +Change weird behavior of PyModule_AddObject() messages: +msg264388 |
| 2016-04-27 12:05:49 | serhiy.storchaka | set | messages: +msg264375 |
| 2016-04-27 09:52:43 | berker.peksag | set | files: +addobject_doc.diff messages: +msg264367 |
| 2016-04-27 07:19:34 | serhiy.storchaka | set | nosy: +serhiy.storchaka messages: +msg264358 |
| 2016-04-27 05:36:44 | berker.peksag | create | |