bpo-41798: Allocate unicodedata CAPI on the heap #24128
Conversation
|
cc @shihai1991 |
Modules/unicodedata.c
Outdated
| PyMem_Free(capi); | ||
| return -1; | ||
| } | ||
| if (PyModule_AddObject(module, "_ucnhash_CAPI", capsule) < 0) { |
There was a problem hiding this comment.
How about using PyModule_AddObjectRef in here too?
There was a problem hiding this comment.
We could do that, but I'm not sure it would improve this code. What do you think?
There was a problem hiding this comment.
PyModule_AddObjectRef() is more easy to understand than PyModule_AddObject(), because PyModule_AddObject() will steal the refs sometimes.
There was a problem hiding this comment.
After thinking about it, I do agree. It's easier to follow the ref count when reading the code. I'll change it. Thanks!
There was a problem hiding this comment.
Should I change the other PyModule_AddObject in unicodedata_exec() while we're there?
|
@vstinner, would you mind reviewing this? |
Not yet. I'm fixing the second regression caused by your latest change :-D https://bugs.python.org/issue42866 I'm not blaming you, regressions are common, and I'm fine with dealing with them. It's just that I prefer to fix all known regressions before taking the risk of adding new ones ;-) |
I totally understand that, and I appreciate you helping out fixing the regressions :) |
|
PTAL, @vstinner |
|
Merged, thanks. |
Thanks for reviewing! |
https://bugs.python.org/issue41798