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

gh-94808: cover PyFunction_GetDefaults and PyFunction_SetDefaults #98449

Merged
merged 2 commits into from
Oct 27, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 19, 2022

@sobolevn
Copy link
Member Author

There's a difference with func_get_defaults:

static PyObject *
func_get_defaults(PyFunctionObject *op, void *Py_UNUSED(ignored))
{
    if (PySys_Audit("object.__getattr__", "Os", op, "__defaults__") < 0) {
        return NULL;
    }
    if (op->func_defaults == NULL) {
        Py_RETURN_NONE;
    }
    Py_INCREF(op->func_defaults);
    return op->func_defaults;
}

{
PyObject *defaults = PyFunction_GetDefaults(func);
if (defaults != NULL) {
Py_INCREF(defaults);
Copy link
Member

Choose a reason for hiding this comment

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

This is correct because PyFunction_GetDefaults returns a borrowed ref.

@JelleZijlstra JelleZijlstra self-assigned this Oct 26, 2022
@JelleZijlstra JelleZijlstra merged commit 7b24333 into python:main Oct 27, 2022
@JelleZijlstra JelleZijlstra added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Oct 27, 2022
@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

Sorry @sobolevn and @JelleZijlstra, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 7b24333fff51f9ca36ef19748016cc2b39ab6301 3.11

@JelleZijlstra JelleZijlstra added needs backport to 3.11 only security fixes and removed needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Oct 27, 2022
@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
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 7b24333fff51f9ca36ef19748016cc2b39ab6301 3.11

@JelleZijlstra
Copy link
Member

@sobolevn (and maybe @mdboom) do you think it's worth manually backporting this, or are we only aiming for full coverage on main?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants