[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


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

[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 813f943c592cf225871b99cffc99304c8cbbee40 by Serhiy Storchaka in 
branch 'master':
bpo-29838: Add asserts for checking results of sq_length and mq_length slots. 
(#700)
https://github.com/python/cpython/commit/813f943c592cf225871b99cffc99304c8cbbee40


--

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

assert() is no-op in release build and that are not critical parts.

Actually the patch decreases the number of checks in release build. Runtime 
check PyErr_Occurred() is moved into assert() in builtin_len().

--

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Raymond Hettinger]

Raymond Hettinger added the comment:

Please don't add more code to critical paths.  

There is no demonstrated need for an additional check here.  The current code 
has worked well for millions of users for many years.

Also, there are too many checks in common paths and some of them are redundant. 
 We need less of this, not more.

--
nosy: +rhettinger

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The alternate approach is raising SystemError rather than crashing in debug 
build on assertion. Since this check is used in exceptional case (when a raised 
exception is expected) it doesn't slow down working code.

--

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Mis-behaving classes have to be written in C. If the __len__ method in Python 
class returns a negative value, ValueError is raised in slot_sq_length().

PySequence_GetItem() already contains an assertion that sq_length returns 
negative value only when an exception is set. The patch just extends this to 
other cases of calling sq_length and mq_length.

--

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Terry J. Reedy]

Terry J. Reedy added the comment:

The changes appear consistent within themselves (ie, no apparent typos), and 
the one for builtin len appears to fix a bug that would allow negative length.  
Can tests be written in Python that fail without the patch?   Or would 
mis-behaving classes have to be written in C?

--
nosy: +terry.reedy

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +574

___
Python tracker 

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

[issue29838] Check that sq_length and mq_length return non-negative result

[Serhiy Storchaka]

New submission from Serhiy Storchaka:

Following PR adds several asserts for checking that sq_length and mq_length 
either return non-negative result or raise an exception.

One assert already was in PySequence_GetItem().

--
components: Interpreter Core
messages: 289777
nosy: haypo, serhiy.storchaka
priority: normal
severity: normal
stage: patch review
status: open
title: Check that sq_length and mq_length return non-negative result
type: enhancement
versions: Python 3.7

___
Python tracker 

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