classification
Title: Inside fcntl module, we does not check the return code of all_ins function
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ezio.melotti, neologix, python-dev, vajrasky, vstinner
Priority: normal Keywords: patch

Created on 2013-09-10 07:08 by vajrasky, last changed 2013-12-01 14:56 by neologix. This issue is now closed.

Files
File name Uploaded Description Edit
check_return_code_all_ins_in_fcntl.patch vajrasky, 2013-09-10 07:08 review
check_return_code_all_ins_in_fcntl_v2.patch vajrasky, 2013-09-15 09:06 review
Messages (5)
msg197425 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-09-10 07:08
In Modules/fcntlmodule.c, there is a code to insert symbolic constants to the module.

    all_ins(m);

But we don't check the return code of the function whether it is successful or not, unlike in posix module in which we check it.

    if (all_ins(m))
        return NULL;

Attached the patch to add checking of the return code of all_ins function in fcntl module file.
msg197756 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-09-15 08:47
I noticed this when converting the code to use PyModule_AddIntMacro().

Vajrasky, did you see Ezio's review on
http://bugs.python.org/review/18994/#ps9258 ?
msg197757 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-09-15 09:06
Charles-François Natali, sorry I just noticed Ezio's comment. The all_ins function return -1 on failure and 0 on success.

I use this form:

    if (all_ins(m))
        return NULL;

because I follow the example in Modules/posixmodule.c. If this form:
    
    if (all_ins(m) < 0)
        return NULL;
    
is better, then so be it, here is the patch to accommodate Ezio's suggestion.
msg204928 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-12-01 14:53
New changeset 84148898a606 by Charles-François Natali in branch 'default':
Issue #18994: Add a missing check for a return value in fcntmodule. Patch by
http://hg.python.org/cpython/rev/84148898a606
msg204929 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-12-01 14:56
Vajrasky, thanks for the patch!
History
Date User Action Args
2013-12-01 14:56:49neologixsetstatus: open -> closed
resolution: fixed
messages: + msg204929

stage: patch review -> resolved
2013-12-01 14:53:50python-devsetnosy: + python-dev
messages: + msg204928
2013-09-15 09:06:36vajraskysetfiles: + check_return_code_all_ins_in_fcntl_v2.patch

messages: + msg197757
2013-09-15 08:47:28neologixsetmessages: + msg197756
2013-09-13 22:06:29vstinnersetnosy: + vstinner
2013-09-13 21:18:38ezio.melottisetnosy: + ezio.melotti, neologix

stage: patch review
2013-09-10 07:08:30vajraskycreate