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: _ctypes\cfield.c identical subexpressions in Z_set
Type: enhancement Stage: resolved
Components: ctypes Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Alexander Riccio, amaury.forgeotdarc, belopolsky, martin.panter, meador.inge, python-dev, random832
Priority: normal Keywords:

Created on 2015-12-12 04:50 by Alexander Riccio, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Messages (5)
msg256256 - (view) Author: Alexander Riccio (Alexander Riccio) * Date: 2015-12-12 04:50
I found this while writing up a separate bug (CPython doesn't use static analysis!).


In _ctypes/cfield.c, Z_set has a bug of some sort:

    if (PyLong_Check(value) || PyLong_Check(value)) {

See: https://hg.python.org/cpython/file/tip/Modules/_ctypes/cfield.c#l1378

...which has been there for at least 5 years: https://hg.python.org/cpython/rev/cab14be0ada1


I'm not sure what the original programmer meant - it's been around forever & I don't know what was there before it - but PyLong_Check(value) is evaluated twice. Which doesn't really make sense.

Compiling with /analyze found this quite easily:

c:\pythondev\repo\modules\_ctypes\cfield.c(1378): warning C6287: Redundant code:  the left and right sub-expressions are identical.



There's a similar issue in P_set, at line 1486.
msg256261 - (view) Author: (random832) Date: 2015-12-12 05:01
Looks like a result of searching and replacing PyInt with PyLong - the diff can be found here, if anyone cares. https://hg.python.org/cpython-fullhistory/rev/f324631462a2.

Oddly, there was _another, older_ revision that fixed this correctly: https://hg.python.org/cpython-fullhistory/rev/003d35215ef2 - looks like there was a bad merge somewhere along the line.
msg256262 - (view) Author: (random832) Date: 2015-12-12 05:10
Sorry, my mistake, I was looking at z_set instead of Z_set. The earlier fix had left Z_set out for some reason. Anyway, just some historical interest anyway.
msg256269 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-12 07:31
New changeset 26859a7e385c by Martin Panter in branch '3.5':
Issue #25845: Drop redundant checks leftover from int to long conversion
https://hg.python.org/cpython/rev/26859a7e385c

New changeset 9be59ad8af80 by Martin Panter in branch 'default':
Issue #25845: Merge PyLong_Check() cleanup from 3.5
https://hg.python.org/cpython/rev/9be59ad8af80
msg256271 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-12 07:38
History is good for understanding how things became the way they are. :) Thanks Alexander and Random!
History
Date User Action Args
2022-04-11 14:58:24adminsetgithub: 70032
2015-12-12 07:38:51martin.pantersetstatus: open -> closed

type: enhancement
versions: + Python 3.5, Python 3.6
nosy: + martin.panter

messages: + msg256271
resolution: fixed
stage: resolved
2015-12-12 07:31:25python-devsetnosy: + python-dev
messages: + msg256269
2015-12-12 05:10:39random832setmessages: + msg256262
2015-12-12 05:01:37random832setnosy: + random832
messages: + msg256261
2015-12-12 04:50:49Alexander Ricciocreate