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

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your review Xiang.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Roundup Robot]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Agreed.

--
assignee:  -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Xiang Zhang]

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 out since _PyLong_Sign also 
checks it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Serhiy Storchaka]

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 2.7, Python 3.5
Added file: http://bugs.python.org/file43437/range_validate_step.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Xiang Zhang]

Changes by Xiang Zhang :


--
components: +Interpreter Core
type:  -> behavior
versions: +Python 2.7, Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Xiang Zhang]

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 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.

--
files: incorrect_logic_in_validate_step.patch
keywords: patch
messages: 268677
nosy: xiang.zhang
priority: normal
severity: normal
status: open
title: validate_step in rangeobject.c, incorrect code logic but right result
Added file: 
http://bugs.python.org/file43419/incorrect_logic_in_validate_step.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com