Skip to content

bpo-1635741: Enhance get_binascii_state() - #19100

Closed
shihai1991 wants to merge 1 commit into
python:masterfrom
shihai1991:bpo_39824_1
Closed

bpo-1635741: Enhance get_binascii_state()#19100
shihai1991 wants to merge 1 commit into
python:masterfrom
shihai1991:bpo_39824_1

Conversation

@shihai1991

@shihai1991shihai1991 commented Mar 21, 2020

Copy link
Copy Markdown
Member

@shihai1991shihai1991 changed the title bpo-1635741: Enhance get_binascii_module()bpo-1635741: Enhance get_binascii_state()Mar 22, 2020

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

Looks good to me.

@corona10

corona10 commented Mar 24, 2020

Copy link
Copy Markdown
Member

Looks like _Pickle_GetState needs same change on sperate PR..

_Pickle_GetState(PyObject*module)

@shihai1991

Copy link
Copy Markdown
MemberAuthor

Looks like _Pickle_GetState needs same change on sperate PR..

_Pickle_GetState(PyObject*module)

I create this sperate change due to binascii extension module have been ported to pep-489.
IMO, other extension modules' get_xxx_state() can be done with porting operation.

@shihai1991

Copy link
Copy Markdown
MemberAuthor

cc @vstinner Hi, victor. It's a enhancement operation of binascii's get_module_state()

@vstinner

Copy link
Copy Markdown
Member

I don't think that this change is worth it. In practice, I don't see why the state would be NULL. If it's the case, the code should crash anyway :-) Existing code which checks that the state is not NULL with an assertion is fine. But I don't think that this change is worth it.

@shihai1991

Copy link
Copy Markdown
MemberAuthor

got it, thanks, victor :)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@shihai1991@corona10@vstinner@the-knights-who-say-ni@bedevere-bot