[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

Serhiy Storchaka added the comment: Thank you for your review Xiang. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

Roundup Robot added the comment: New changeset 4fbcd58df1a0 by Serhiy Storchaka in branch 'default': Issue #27333: Simplified testing step on 0. https://hg.python.org/cpython/rev/4fbcd58df1a0 -- nosy: +python-dev ___ Python tracker

[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

Serhiy Storchaka added the comment: Agreed. -- assignee: -> serhiy.storchaka ___ Python tracker ___ ___

[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

Xiang Zhang added the comment: 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

[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

Serhiy Storchaka added the comment: 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. -- nosy: +mark.dickinson stage: -> patch review type: behavior -> enhancement versions: -Python

[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

Changes by Serhiy Storchaka : -- nosy: +serhiy.storchaka ___ Python tracker ___

[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

Changes by Xiang Zhang : -- components: +Interpreter Core type: -> behavior versions: +Python 2.7, Python 3.5, Python 3.6 ___ Python tracker

[issue27333] validate_step in rangeobject.c, incorrect code logic but right result

New submission from Xiang Zhang: 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