◐ Shell
reader mode source ↗
Skip to content

bpo-1635741: Calling Py_INCREF() after PyModule_AddObject() success to run#18365

Closed
shihai1991 wants to merge 2 commits into
python:masterfrom
shihai1991:bpo_1635741_10
Closed

bpo-1635741: Calling Py_INCREF() after PyModule_AddObject() success to run#18365
shihai1991 wants to merge 2 commits into
python:masterfrom
shihai1991:bpo_1635741_10

Conversation

@shihai1991

@shihai1991 shihai1991 commented Feb 5, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@shihai1991

shihai1991 commented Feb 5, 2020

Copy link
Copy Markdown
Member Author

Oh, it is generated file, i will check this operation soon.

@shihai1991 shihai1991 changed the title bpo-1635741: Calling Py_DECREF() if PyModule_AddObject() fails to run Feb 5, 2020
@shihai1991

shihai1991 commented Feb 5, 2020

Copy link
Copy Markdown
Member Author

cc @vstinner Hi, victor. It's a small fix. pls review it if you have free time.

@shihai1991 shihai1991 requested a review from vstinner February 5, 2020 14:26
@vstinner

vstinner commented Feb 5, 2020

Copy link
Copy Markdown
Member

cc @pablogsal @isidentical

@isidentical

isidentical commented Feb 5, 2020

Copy link
Copy Markdown
Member

I recently saw a PR from @brandtbucher about a more general change that already includes this if I am not mistaken. Check out #17276 (it moves incref after the check by moving check upper). I am in favor of #17276 because it handles the case better also it opened before this PR

@brandtbucher

brandtbucher commented Feb 5, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I'm also working my way through the other broken modules in bpo-38823. I need to land #17298 before I can make further progress, though.

brandtbucher
brandtbucher requested changes Feb 5, 2020
View reviewed changes

@brandtbucher brandtbucher 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.

Hide comment

Yes, my PR #17276 is a more comprehensive fix.

@brandtbucher brandtbucher added the type-bug An unexpected behavior, bug, or error label Feb 5, 2020
@shihai1991

shihai1991 commented Feb 6, 2020

Copy link
Copy Markdown
Member Author

Oh, thank you for quick feedback, guys.
I checked Brandt's PRs, its enhance very nice. Pls go ahead, thanks. I will subscribe your PRs ;)

@shihai1991 shihai1991 closed this Feb 6, 2020
@kylotan kylotan mannequin mentioned this pull request Sep 19, 2022
Closed
@brandtbucher brandtbucher mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandtbucher brandtbucher brandtbucher requested changes

@vstinner vstinner

Assignees

No one assigned

Labels

awaiting core review type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@shihai1991 @vstinner @isidentical @brandtbucher @the-knights-who-say-ni @bedevere-bot