[proxy] web.archive.org← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light

/cpython

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-38142: Updated _hashopenssl.c to be PEP 384 compliant #16071

Merged
merged 4 commits into from Sep 25, 2019

Conversation

Copy link
Member

tiran commented Sep 12, 2019

The _hashlib OpenSSL wrapper extension module is now PEP 384 compliant.

Also remove refleak test from test_hashlib. The updated type no longer accepts random arguments to __init__.

https://bugs.python.org/issue38142

Copy link
Contributor

eduardo-elizondo left a comment

Since EVP is now a heap allocated type, you should now properly refcount the type as well. You can read the details in: https://docs.python.org/3.8/whatsnew/3.8.html#changes-in-the-c-api, search for bpo-35810

Anyways, the TL;DR is that EVP_dealloc should now be:

static void
EVP_dealloc(EVPobject *self)
{
    PyTypeObject *tp = Py_TYPE(self);
    if (self->lock != NULL)
        PyThread_free_lock(self->lock);
    EVP_MD_CTX_free(self->ctx);
    PyObject_Del(self);
    Py_DECREF(tp);
}
ericsnowcurrently self-requested a review Sep 13, 2019
tiran force-pushed the tiran:bpo-38142-hashlib-mod branch from 945e8e2 to aa25666 Sep 13, 2019

This comment has been minimized.

Copy link
Member Author

tiran commented Sep 13, 2019

Since EVP is now a heap allocated type, you should now properly refcount the type as well. You can read the details in: docs.python.org/3.8/whatsnew/3.8.html#changes-in-the-c-api, search for bpo-35810

Thanks! I missed the note. :)

Copy link
Contributor

eduardo-elizondo left a comment

Perfect! This LGTM now!

This comment has been minimized.

Copy link
Contributor

eduardo-elizondo commented Sep 13, 2019

Agh I forget I don't have the commit bit here. Let's ping someone else to merge, cc @ericsnowcurrently @DinoV

tiran added 4 commits Sep 12, 2019
Signed-off-by: Christian Heimes <christian@python.org>
The updated type no longer accepts random arguments to __init__.

Signed-off-by: Christian Heimes <christian@python.org>
Signed-off-by: Christian Heimes <christian@python.org>
Signed-off-by: Christian Heimes <christian@python.org>
tiran force-pushed the tiran:bpo-38142-hashlib-mod branch from 6e95139 to a4b8323 Sep 25, 2019
tiran merged commit df69e75 into python:master Sep 25, 2019
4 checks passed
4 checks passed
Azure Pipelines PR #20190925.47 succeeded
Details
bedevere/issue-number Issue number 38142 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
tiran deleted the tiran:bpo-38142-hashlib-mod branch Sep 25, 2019

This comment has been minimized.

Copy link

bedevere-bot commented Sep 25, 2019

@tiran: Please replace # with GH- in the commit message next time. Thanks!

miss-islington added a commit that referenced this pull request Sep 27, 2019
…H-16248)

As mentioned in the bpo ticket, this mistake came up on two reviews:
- #16127 (review)
- #16071 (review)

Would be nice to have it documented in a more permanent place than 3.8's whatsnew entry.


https://bugs.python.org/issue38206



Automerge-Triggered-By: @encukou
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants