-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-106168: Check allocated instead of size index bounds in PyList_SET_ITEM() #111480
Conversation
…) against [0:allocated] instead of [0:size] to re-allow valid use cases that assign within the allocated area.
I'm not sure what to make of the recommendation in the "what's new" paragraph. I specifically wouldn't recommend setting the size first, before setting the value. That seems incorrect and fragile. I'd remove it, but that'd leave us entirely without a recommendation. |
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
With this change,
Well, here the problem is not about designing a correct API, but to migrate Python 3.12 code to Python 3.13. Previously, |
Since Python 3.13.0 final was not released, a Changelog entry is enough to document changes since 3.13 alpha1 (a NEWS.d entry added by the blurb tool). |
I merged your change without Changelog entry, I don't think that it's needed for such subtile change. Thanks. |
I created issue #111489: [C API] Make _PyList_FromArraySteal() function public. |
I was referring to the existing whats-new paragraph, which says "if the assertion fails, set the size first". That is no longer true. |
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.
I don't see an inconsistency here. They serve different needs, that's why we have two different functions. As you write, You could rather argue that Cython uses the |
Ah oops, I tried to find a What's New entry, but I failed to find it, and so made the assumption that I didn't document the change. It should be updated, you're right. |
I wrote PR #111618 to update What's New in Python 3.13. |
…st_SET_ITEM() (python#111480) Check the index bound assertions in PyList_SET_ITEM() against [0:allocated] instead of [0:size] to re-allow valid use cases that assign within the allocated area.
…st_SET_ITEM() (python#111480) Check the index bound assertions in PyList_SET_ITEM() against [0:allocated] instead of [0:size] to re-allow valid use cases that assign within the allocated area.
…st_SET_ITEM() (python#111480) Check the index bound assertions in PyList_SET_ITEM() against [0:allocated] instead of [0:size] to re-allow valid use cases that assign within the allocated area.
Check the index bound assertions in PyList_SET_ITEM() against [0:allocated] instead of [0:size] to re-allow valid use cases that assign within the allocated area.
First increasing the size before setting a new value seems less safe and clean than allowing access to the allocated area and adjusting the size on successful updates.