bpo-41798: Allocate socket extension module CAPI on the heap #24126
Conversation
|
cc. @tiran / @shihai1991 |
| Py_DECREF(m); | ||
| return NULL; | ||
| } | ||
| if (PyModule_AddObject(m, PySocket_CAPI_NAME, capsule) < 0) { |
There was a problem hiding this comment.
How about using PyModule_AddObjectRef() in here?
There was a problem hiding this comment.
I think we can leave it as it is for now. We can do a thorough clean up of the module init function when we convert it to multi-phase init. Ok?
| @@ -342,7 +342,8 @@ typedef struct { | |||
|
|
|||
| */ | |||
|
|
|||
| /* C API for usage by other Python modules */ | |||
| /* C API for usage by other Python modules. | |||
| * Always add new things to the end for binary compatibility. */ | |||
There was a problem hiding this comment.
I am not sure this note is correct or not. The binary compatibilty also depends how to use it, no?
There was a problem hiding this comment.
You're right. I just copied the comment from the .c file. I figured it made more sense together with the definition in the header file. However, I don't think this comment is really needed. I'd recommend just deleting it.
There was a problem hiding this comment.
In case of doubt, I prefer to keep it. It makes sense to only add new members at the end. If a C extension is built with Python 3.10 and a new filed is added to Python 3.11, the Python 3.10 remains ABI compatible. The comment looks correct.
| @@ -342,7 +342,8 @@ typedef struct { | |||
|
|
|||
| */ | |||
|
|
|||
| /* C API for usage by other Python modules */ | |||
| /* C API for usage by other Python modules. | |||
| * Always add new things to the end for binary compatibility. */ | |||
There was a problem hiding this comment.
In case of doubt, I prefer to keep it. It makes sense to only add new members at the end. If a C extension is built with Python 3.10 and a new filed is added to Python 3.11, the Python 3.10 remains ABI compatible. The comment looks correct.
https://bugs.python.org/issue41798