bpo-20182: AC convert remaining functions/methods in _hashopenssl.c #9213
Conversation
Modules/_hashopenssl.c
Outdated
| #include "clinic/_hashopenssl.c.h" | ||
| /*[clinic input] | ||
| module _hashlib | ||
| class _hashlib.HASH "EVPobject *" "& EVPtype" |
There was a problem hiding this comment.
Nit: it is uncommon to add a space after &.
| @@ -329,24 +345,37 @@ EVP_repr(EVPobject *self) | |||
| return PyUnicode_FromFormat("<%U HASH object @ %p>", self->name, self); | |||
| } | |||
|
|
|||
| #if HASH_OBJ_CONSTRUCTOR | |||
There was a problem hiding this comment.
Why this line was removed?
There was a problem hiding this comment.
So that the doc-string would still be generated, as it is used as the class's doc-string.
The init method is still not used, since its inclusion in the PyTypeObject definition is still conditional on HASH_OBJ_CONSTRUCTOR.
Modules/_hashopenssl.c
Outdated
| _hashlib.HASH.__init__ as EVP_tp_init | ||
|
|
||
| name as name_obj: object | ||
| string as data_obj: object = NULL |
There was a problem hiding this comment.
Does passing an empty bytes object have the same effect as omitting the second argument? If yes, I suggest to use b'' as Python default.
Modules/_hashopenssl.c
Outdated
| The MD5 and SHA1 algorithms are always supported.\n"); | ||
| /*[clinic input] | ||
| @classmethod | ||
| _hashlib.HASH.__new__ as EVP_new |
There was a problem hiding this comment.
It is not _hashlib.HASH.__new__, it is _hashlib.new.
|
@serhiy-storchaka, thanks for the review! I've addressed your (excellent) comments and would be happy for another review. |
|
I removed the skipnews label. Only trivial PRs are allowed to skip news. @vstinner, @encukou, @serhiy-storchaka something is wrong with CODEOWNERS for the file. Any change to the hash module should be gated by @gpshead, Alex, or me. |
How is Or perhaps |
|
It's a mystery. @python/crypto-team exists and I'm a member. |
|
Can confirm it works well enough that I got an email :D |
|
I think that the NEWS file should contain only information that is interested for end users. Refactorings and conversions to Argument Clinic are not mentioned in it. |
|
@tiran, I'd like to re-apply the "skip news" label and merge this. Are you okay with that? |
|
I've added the skip news label for you (nothing news worthy about this internal refactoring of _hashopenssl.c). This PR likely needs syncing with merge conflicts addressed before it can go in. |
|
I'd gladly resolve the conflicts and merge this in. I was waiting for @tiran to approve this; but given that @serhiy-storchaka and @gpshead have approved I guess this is good to go. |
Notes:
_hashlib.HASHmethods, e.g._hashlib.HASH.copy, despite them not usingPyArg_Parse*argument parsing code.#include "clinic/_hashopenssl.c.h"line had to be moved down a bit in the file to resolve a compilation issue.https://bugs.python.org/issue20182