bpo-34190: Fix reference leak in call_function()#8413
Conversation
There was a problem hiding this comment.
Hum, I propose to be more explicit here:
}
else {
x = NULL;
}
instead of always initializing x to NULL.
vstinner
left a comment
There was a problem hiding this comment.
LGTM, but I proposed a minor coding style change.
|
Personally, I think it's better style to initialize |
|
Don't forget to add the backport to 3.7 label. |
|
Backport to 3.7, not 3.6 (which doesn't contain the bug yet) |
There was a problem hiding this comment.
nitpick: please exchange the two branches, first the common func != NULL case.
There was a problem hiding this comment.
first the common func != NULL case.
Where in PEP 7 does it say that the most common case in an if should be first?
There was a problem hiding this comment.
There is no guide line for that. It's just my preference.
The compiler complains if we use x without initializing it. I asked to not initialize x to help the compiler to emit the most efficient code, since call_function() is critical to Python performances. |
If your compiler is unable to optimize useless assignments, get a better compiler. Seriously, the compiler is generally better at optimizing code than you are. |
(cherry picked from commit 147d955) Co-authored-by: jdemeyer <jdemeyer@cage.ugent.be>
|
GH-8418 is a backport of this pull request to the 3.7 branch. |
Needs backport to 3.7
I'm not sure whether this needs a
NEWSentry. While the bug existed before #8300, one could argue that this is really just an improvement to #8300.https://bugs.python.org/issue34190