Movatterモバイル変換


[0]ホーム

URL:


homepage

Issue26868

This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title:Document PyModule_AddObject's behavior on error
Type:behaviorStage:patch review
Components:Extension ModulesVersions:Python 3.8, Python 3.7, Python 3.6
process
Status:openResolution:
Dependencies:26871Superseder:
Assigned To:Nosy List: berker.peksag, brandtbucher, malin, matrixise, miss-islington, nirs, serhiy.storchaka
Priority:lowKeywords:patch

Created on2016-04-27 05:36 byberker.peksag, last changed2022-04-11 14:58 byadmin.

Files
File nameUploadedDescriptionEdit
csv.diffberker.peksag,2016-04-27 05:36review
addobject_doc.diffberker.peksag,2016-04-27 09:52review
issue26868_v2.diffberker.peksag,2016-06-10 05:37review
Pull Requests
URLStatusLinkedEdit
PR 15725mergedbrandtbucher,2019-09-07 15:52
PR 16037mergedmiss-islington,2019-09-12 12:11
PR 16038mergedmiss-islington,2019-09-12 12:11
Messages (12)
msg264350 -(view)Author: Berker Peksag (berker.peksag)*(Python committer)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)*(Python committer)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)*(Python committer)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)*(Python committer)Date: 2016-04-27 12:05
Added comments on Rietveld.
msg264388 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)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)*(Python committer)Date: 2016-06-10 05:37
Thanks for the review Serhiy. Here is an updated patch.
msg276977 -(view)Author: Berker Peksag (berker.peksag)*(Python committer)Date: 2016-09-19 13:29
Serhiy, do you have further comments about issue26868_v2.diff?
msg330940 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)Date: 2018-12-03 11:53
issue26868_v2.diff LGTM.
msg351260 -(view)Author: Brandt Bucher (brandtbucher)*(Python committer)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)*(Python committer)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
DateUserActionArgs
2022-04-11 14:58:30adminsetgithub: 71055
2019-09-12 12:29:12miss-islingtonsetmessages: +msg352141
2019-09-12 12:26:49miss-islingtonsetnosy: +miss-islington
messages: +msg352140
2019-09-12 12:11:39miss-islingtonsetpull_requests: +pull_request15660
2019-09-12 12:11:32miss-islingtonsetpull_requests: +pull_request15659
2019-09-12 12:11:23matrixisesetnosy: +matrixise
messages: +msg352135
2019-09-07 23:54:25malinsetnosy: +malin
2019-09-07 15:52:11brandtbuchersetpull_requests: +pull_request15380
2019-09-06 15:57:27brandtbuchersetnosy: +brandtbucher
messages: +msg351260
2019-04-28 22:41:04nirssetnosy: +nirs
2018-12-03 11:53:26serhiy.storchakasetversions: + Python 3.8, - Python 3.5
2018-12-03 11:53:16serhiy.storchakasetmessages: +msg330940
2016-09-19 13:29:00berker.peksagsetmessages: +msg276977
versions: + Python 3.7
2016-06-10 05:37:40berker.peksagsetfiles: +issue26868_v2.diff

messages: +msg268088
2016-04-28 15:31:14berker.peksagsettitle: Incorrect check for return value of PyModule_AddObject in _csv.c -> Document PyModule_AddObject's behavior on error
2016-04-27 18:09:03serhiy.storchakasetdependencies: +Change weird behavior of PyModule_AddObject()
messages: +msg264388
2016-04-27 12:05:49serhiy.storchakasetmessages: +msg264375
2016-04-27 09:52:43berker.peksagsetfiles: +addobject_doc.diff

messages: +msg264367
2016-04-27 07:19:34serhiy.storchakasetnosy: +serhiy.storchaka
messages: +msg264358
2016-04-27 05:36:44berker.peksagcreate
Supported byThe Python Software Foundation,
Powered byRoundup
Copyright © 1990-2022,Python Software Foundation
Legal Statements

[8]ページ先頭

©2009-2026 Movatter.jp