FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-110241: Add missing error check to `record_eval` in `_testinternalcapi` by sobolevn · Pull Request #110242 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

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

Merged
JelleZijlstra merged 1 commit into
python:mainfrom
sobolevn:issue-110241
Oct 2, 2023
ConversationCommits (1)ChecksFiles changed
Merged

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

Conversation

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

Copy link
Copy Markdown
Member

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

JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 enabled auto-merge (squash) October 2, 2023 20:54

sobolevn commented Oct 2, 2023

Copy link
Copy Markdown
Member Author

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 merged commit 4596c76 into python:main Oct 2, 2023

miss-islington commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

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 commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

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
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 Bot commented Oct 2, 2023

Copy link
Copy Markdown

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

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 2, 2023

JelleZijlstra commented Oct 2, 2023

Copy link
Copy Markdown
Member

Guess this code isn't in 3.11.

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
[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
pythongh-110241: Add missing error check to record_eval in `_testin…
f92985b 
…ternalcapi` (python#110242)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JelleZijlstra JelleZijlstra approved these changes

Assignees

JelleZijlstra

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL