Skip to content
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

bpo-36796: Clean the error handling in _testcapimodule.c #13085

Merged

Conversation

ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented May 4, 2019

In addition, fix possible ref counting bugs in test_L_code() and
test_k_code() -- it was possible for "num" to be decrefed twice.

https://bugs.python.org/issue36796

In addition, fix possible ref counting bugs in test_L_code() and
test_k_code() -- it was possible for "num" to be decrefed twice.
@pganssle
Copy link
Member

pganssle commented May 4, 2019

The timezone portion of the changes looks good to me, haven't looked at the other parts.

Modules/_testcapimodule.c Outdated Show resolved Hide resolved
Modules/_testcapimodule.c Outdated Show resolved Hide resolved

PyTuple_SET_ITEM(tuple, 0, num);
PyTuple_SetItem(tuple, 0, num);
Copy link
Member

Choose a reason for hiding this comment

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

Why it has been changed?

Modules/_testcapimodule.c Outdated Show resolved Hide resolved
Modules/_testcapimodule.c Outdated Show resolved Hide resolved
@ZackerySpytz
Copy link
Contributor Author

Hi, @serhiy-storchaka. Thank you for the review.

This PR deals solely with missing failure checks and possible ref counting bugs. If it is desired to make other changes to some functions in _testcapimodule.c, then perhaps that should be handled in a separate issue.

@matrixise
Copy link
Member

@serhiy-storchaka do you continue to review the PR of @ZackerySpytz ? Thank you

@serhiy-storchaka serhiy-storchaka added skip news needs backport to 3.12 bug and security fixes tests Tests in the Lib/test dir labels Dec 14, 2023
@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) December 14, 2023 18:44
@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) December 14, 2023 18:45
@serhiy-storchaka serhiy-storchaka merged commit a723a13 into python:main Dec 14, 2023
35 checks passed
@miss-islington-app
Copy link

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@bedevere-app
Copy link

bedevere-app bot commented Dec 14, 2023

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 14, 2023
)

(cherry picked from commit a723a13)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Dec 14, 2023
serhiy-storchaka pushed a commit that referenced this pull request Dec 14, 2023
…3085) (GH-113132)

(cherry picked from commit a723a13)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
corona10 pushed a commit to corona10/cpython that referenced this pull request Dec 15, 2023
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants