Repository navigation
C API: Check in PyTuple_SET_ITEM() and PyList_SET_ITEM() #106168
Description
Activity
- added 5 commits that reference this issue
on Jun 28, 2023 The PyTuple_SET_ITEM() and PyList_SET_ITEM() functions don't check that the index is valid. It should be checked with an assertion.
I oppose this statement. Their documentation says that both allow unchecked writing into the list/tuple. Adding these checks breaks their contract.
If you insist on adding such a check, you can assert that the index is within the currently allocated array bounds. Writing outside of that range is definitely incorrect. However, writing beyond the current size may not be.
- added a commit that references this issue
on Oct 30, 2023 [Copying my comment from the PR here as IMHO it describes the situation quite well and thus belongs rather into the ticket discussion.]
@vstinner wrote:
I dislike PyList_New() + PyList_SET_ITEM() API since the list is immediately tracked by the GC and so calling gc.get_objects() can expose an invalid list object (ex: calling
repr(list)can crash)That's why I prefer not requiring users to change the size before they set the value. Doing that would bring the list in an invalid state.
With this change,
PyList_SetItem()andPyList_SET_ITEM()become inconsistent:PyList_SetItem()useslist->ob_size, whereasPyList_SET_ITEM()useslist->allocated.I don't see an inconsistency here. They serve different needs, that's why we have two different functions. As you write,
PyList_SetItem()decrefs the current value and replaces it with a new one. That can only safely be done within [0:size].PyList_SET_ITEM()writes a value to an array position that does not yet have a value. That can only safely be done within [0:allocated]. Different needs, different functions, different boundary conditions.You could rather argue that
PyList_SET_ITEM()is only really valid within [size:allocated]. Within [0:size], it would overwrite existing, owned values. However, enforcing that would probably break even more code that might just look slightly brittle but is actually working perfectly fine (because it does what you suggested, it updates the size immediately before setting the value).Cython uses the
_SET_ITEM()functions to initialise freshly created lists and tuples, but it also callsPyList_SET_ITEM()in a fast, inlinedPyList_Append()implementation:
https://github.com/cython/cython/blob/master/Cython/Utility/Optimize.c#L29-L77
We use this for list comprehensions, for example, where we control the list internally. As long as the next item fits into the already allocated array, we just put it there and increase the size.- added a commit that references this issue
on Nov 3, 2023
The PyTuple_SET_ITEM() and PyList_SET_ITEM() functions don't check that the index is valid. It should be checked with an assertion.
Linked PRs