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: validate_step in rangeobject.c, incorrect code logic but right result
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: mark.dickinson, python-dev, serhiy.storchaka, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-06-16 16:36 by xiang.zhang, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
incorrect_logic_in_validate_step.patch xiang.zhang, 2016-06-16 16:36 review
range_validate_step.patch serhiy.storchaka, 2016-06-17 13:04 review
Messages (6)
msg268677 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-16 16:36
Here is a drawback in validate_step of rangeobject. PyNumber_AsSsize_t returns clipped value and won't set an exception when the argument *exc* is set NULL. So if step overflows, istep is always PY_SSIZE_MAX or PY_SSIZE_MIN. But the following code is to check if istep is -1 and there is an exception. The code actually conflicts. But fortunately the result is always right. I suggest to make the code logic right.
msg268723 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-17 13:04
Current code is valid, since neither PY_SSIZE_MAX nor PY_SSIZE_MIN equal to 0.

But testing on 0 can be simpler. Following patch simplifies the code.
msg268728 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-17 15:17
This looks fine. But maybe code like this looks more clear:

if (step && _PyLong_Sign(step) == 0) {
    PyErr_SetString(PyExc_ValueError,
                    "range() arg 3 must not be zero");
    Py_CLEAR(step);
}

I think assert(PyLong_Check(step)) can be left out since _PyLong_Sign also checks it.
msg268769 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-18 06:46
Agreed.
msg268770 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-18 06:52
New changeset 4fbcd58df1a0 by Serhiy Storchaka in branch 'default':
Issue #27333: Simplified testing step on 0.
https://hg.python.org/cpython/rev/4fbcd58df1a0
msg268771 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-18 06:53
Thank you for your review Xiang.
History
Date User Action Args
2022-04-11 14:58:32adminsetgithub: 71520
2016-06-18 06:53:38serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg268771

stage: patch review -> resolved
2016-06-18 06:52:50python-devsetnosy: + python-dev
messages: + msg268770
2016-06-18 06:46:11serhiy.storchakasetassignee: serhiy.storchaka
messages: + msg268769
2016-06-17 15:17:51xiang.zhangsetmessages: + msg268728
2016-06-17 13:04:24serhiy.storchakasetfiles: + range_validate_step.patch

type: behavior -> enhancement
versions: - Python 2.7, Python 3.5
nosy: + mark.dickinson

messages: + msg268723
stage: patch review
2016-06-17 08:38:44serhiy.storchakasetnosy: + serhiy.storchaka
2016-06-16 16:41:04xiang.zhangsettype: behavior
components: + Interpreter Core
versions: + Python 2.7, Python 3.5, Python 3.6
2016-06-16 16:36:25xiang.zhangcreate