Skip to content

gh-110241: Add missing error check to record_eval in _testinternalcapi#110242

Merged
JelleZijlstra merged 1 commit intopython:mainfrom
sobolevn:issue-110241
Oct 2, 2023
Merged

gh-110241: Add missing error check to record_eval in _testinternalcapi#110242
JelleZijlstra merged 1 commit intopython:mainfrom
sobolevn:issue-110241

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 2, 2023 •
edited by bedevere-app bot
Loading

@sobolevn sobolevn requested a review from JelleZijlstra October 2, 2023 20:48
@bedevere-app bedevere-app bot mentioned this pull request Oct 2, 2023
Closed
JelleZijlstra
JelleZijlstra approved these changes Oct 2, 2023
View reviewed changes
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I opened that issue I half expected you to grep and find more cases :).

I take it you grepped for calls to PyList_Append where the return value is ignored and this was the only other one?

@JelleZijlstra JelleZijlstra enabled auto-merge (squash) October 2, 2023 20:54
@sobolevn
Copy link
Member Author

sobolevn commented Oct 2, 2023

I take it you grepped for calls to PyList_Append where the return value is ignored and this was the only other one?

Yeap :)

There are some PyList_SetItem that I found, but it is too late today. I will do that tomorrow. It also requires some research: either using PyList_SET_ITEM or proper error handling.

@JelleZijlstra JelleZijlstra merged commit 4596c76 into python:main Oct 2, 2023
@miss-islington
Copy link
Contributor

miss-islington commented Oct 2, 2023

Thanks @sobolevn for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Oct 2, 2023

Sorry, @sobolevn and @JelleZijlstra, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 4596c76d1a7650fd4650c814dc1d40d664cd8fb4 3.11

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 2, 2023
@sobolevn @miss-islington
pythongh-110241: Add missing error check to record_eval in `_testin…
4431a58 
…ternalcapi` (pythonGH-110242)

(cherry picked from commit 4596c76)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-app
Copy link

bedevere-app bot commented Oct 2, 2023

GH-110244 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 2, 2023
@JelleZijlstra
Copy link
Member

JelleZijlstra commented Oct 2, 2023

Guess this code isn't in 3.11.

@JelleZijlstra JelleZijlstra removed the needs backport to 3.11 only security fixes label Oct 2, 2023
JelleZijlstra pushed a commit that referenced this pull request Oct 2, 2023
@miss-islington @sobolevn
[3.12] gh-110241: Add missing error check to record_eval in `_testi…
892b194 
…nternalcapi` (GH-110242) (#110244)

gh-110241: Add missing error check to `record_eval` in `_testinternalcapi` (GH-110242)
(cherry picked from commit 4596c76)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@sobolevn @Glyphack
pythongh-110241: Add missing error check to record_eval in `_testin…
f92985b 
…ternalcapi` (python#110242)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

Assignees

@JelleZijlstra JelleZijlstra

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sobolevn @miss-islington @JelleZijlstra