From e765d78a1e2de226ba112abd07d8615d0140c142 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 3 Nov 2023 08:58:40 +0100 Subject: [PATCH] refactor: Port to multi-phase module init The `_ldap` module now uses modern multi-phase module initialization. Replace `LDAPadd_methods` hack with proper PyMethodDef for module-level functions. The old approache is incompatible with multi-phase init. Module-level functions are now prefixed with `LDAPMod_` and exported. Use `PyModuleDef_Slot` to initialize the `_ldap` C extension. See: https://github.com/python-ldap/python-ldap/issues/540 Signed-off-by: Christian Heimes --- Misc/python-ldap.supp | 2 +- Modules/LDAPObject.c | 10 ++++++ Modules/common.c | 15 --------- Modules/constants.c | 2 +- Modules/functions.c | 39 ++++++---------------- Modules/ldapcontrol.c | 30 +++++------------ Modules/ldapmodule.c | 77 +++++++++++++++++++++---------------------- Modules/pythonldap.h | 22 +++++++++---- 8 files changed, 82 insertions(+), 115 deletions(-) diff --git a/Misc/python-ldap.supp b/Misc/python-ldap.supp index b9954a9a..8384ac90 100644 --- a/Misc/python-ldap.supp +++ b/Misc/python-ldap.supp @@ -46,7 +46,7 @@ fun:PyObject_Malloc ... fun:PyErr_NewException - fun:LDAPinit_constants + fun:LDAPMod_init_constants fun:init_ldap_module ... } diff --git a/Modules/LDAPObject.c b/Modules/LDAPObject.c index 71fac73e..fb0dc40b 100644 --- a/Modules/LDAPObject.c +++ b/Modules/LDAPObject.c @@ -1539,3 +1539,13 @@ PyTypeObject LDAP_Type = { 0, /*tp_members */ 0, /*tp_getset */ }; + +int +LDAPMod_init_type(PyObject *m) +{ + /* Initialize LDAP class */ + if (PyType_Ready(&LDAP_Type) < 0) { + return -1; + } + return 0; +} diff --git a/Modules/common.c b/Modules/common.c index 4cfee744..e9bf789c 100644 --- a/Modules/common.c +++ b/Modules/common.c @@ -3,21 +3,6 @@ #include "pythonldap.h" -/* dynamically add the methods into the module dictionary d */ - -void -LDAPadd_methods(PyObject *d, PyMethodDef *methods) -{ - PyMethodDef *meth; - - for (meth = methods; meth->ml_meth; meth++) { - PyObject *f = PyCFunction_New(meth, NULL); - - PyDict_SetItemString(d, meth->ml_name, f); - Py_DECREF(f); - } -} - /* Raise TypeError with custom message and object */ PyObject * LDAPerror_TypeError(const char *msg, PyObject *obj) diff --git a/Modules/constants.c b/Modules/constants.c index f0a0da94..47b045ca 100644 --- a/Modules/constants.c +++ b/Modules/constants.c @@ -192,7 +192,7 @@ LDAPerror(LDAP *l) /* initialise the module constants */ int -LDAPinit_constants(PyObject *m) +LDAPMod_init_constants(PyObject *m) { PyObject *exc, *nobj; struct ldap_apifeature_info info = { 1, "X_OPENLDAP_THREAD_SAFE", 0 }; diff --git a/Modules/functions.c b/Modules/functions.c index f7d9cf37..23cd7acb 100644 --- a/Modules/functions.c +++ b/Modules/functions.c @@ -4,8 +4,8 @@ /* ldap_initialize */ -static PyObject * -l_ldap_initialize(PyObject *unused, PyObject *args) +PyObject * +LDAPMod_initialize(PyObject *module, PyObject *args) { char *uri; LDAP *ld = NULL; @@ -28,8 +28,8 @@ l_ldap_initialize(PyObject *unused, PyObject *args) #ifdef HAVE_LDAP_INIT_FD /* initialize_fd(fileno, url) */ -static PyObject * -l_ldap_initialize_fd(PyObject *unused, PyObject *args) +PyObject * +LDAPMod_initialize_fd(PyObject *module, PyObject *args) { char *url; LDAP *ld = NULL; @@ -82,8 +82,8 @@ l_ldap_initialize_fd(PyObject *unused, PyObject *args) /* ldap_str2dn */ -static PyObject * -l_ldap_str2dn(PyObject *unused, PyObject *args) +PyObject * +LDAPMod_str2dn(PyObject *module, PyObject *args) { struct berval str; LDAPDN dn; @@ -157,8 +157,8 @@ l_ldap_str2dn(PyObject *unused, PyObject *args) /* ldap_set_option (global options) */ -static PyObject * -l_ldap_set_option(PyObject *self, PyObject *args) +PyObject * +LDAPMod_set_option(PyObject *module, PyObject *args) { PyObject *value; int option; @@ -173,8 +173,8 @@ l_ldap_set_option(PyObject *self, PyObject *args) /* ldap_get_option (global options) */ -static PyObject * -l_ldap_get_option(PyObject *self, PyObject *args) +PyObject * +LDAPMod_get_option(PyObject *module, PyObject *args) { int option; @@ -184,22 +184,3 @@ l_ldap_get_option(PyObject *self, PyObject *args) } /* methods */ - -static PyMethodDef methods[] = { - {"initialize", (PyCFunction)l_ldap_initialize, METH_VARARGS}, -#ifdef HAVE_LDAP_INIT_FD - {"initialize_fd", (PyCFunction)l_ldap_initialize_fd, METH_VARARGS}, -#endif - {"str2dn", (PyCFunction)l_ldap_str2dn, METH_VARARGS}, - {"set_option", (PyCFunction)l_ldap_set_option, METH_VARARGS}, - {"get_option", (PyCFunction)l_ldap_get_option, METH_VARARGS}, - {NULL, NULL} -}; - -/* initialisation */ - -void -LDAPinit_functions(PyObject *d) -{ - LDAPadd_methods(d, methods); -} diff --git a/Modules/ldapcontrol.c b/Modules/ldapcontrol.c index 4a37b614..aa099217 100644 --- a/Modules/ldapcontrol.c +++ b/Modules/ldapcontrol.c @@ -194,8 +194,8 @@ LDAPControls_to_List(LDAPControl **ldcs) /* --------------- en-/decoders ------------- */ /* Matched Values, aka, Values Return Filter */ -static PyObject * -encode_rfc3876(PyObject *self, PyObject *args) +PyObject * +LDAPMod_encode_rfc3876(PyObject *module, PyObject *args) { PyObject *res = 0; int err; @@ -235,8 +235,8 @@ encode_rfc3876(PyObject *self, PyObject *args) return res; } -static PyObject * -encode_rfc2696(PyObject *self, PyObject *args) +PyObject * +LDAPMod_encode_rfc2696(PyObject *module, PyObject *args) { PyObject *res = 0; BerElement *ber = 0; @@ -291,8 +291,8 @@ encode_rfc2696(PyObject *self, PyObject *args) return res; } -static PyObject * -decode_rfc2696(PyObject *self, PyObject *args) +PyObject * +LDAPMod_decode_rfc2696(PyObject *module, PyObject *args) { PyObject *res = 0; BerElement *ber = 0; @@ -328,8 +328,8 @@ decode_rfc2696(PyObject *self, PyObject *args) return res; } -static PyObject * -encode_assertion_control(PyObject *self, PyObject *args) +PyObject * +LDAPMod_encode_assertion_control(PyObject *module, PyObject *args) { int err; PyObject *res = 0; @@ -374,17 +374,3 @@ encode_assertion_control(PyObject *self, PyObject *args) return res; } - -static PyMethodDef methods[] = { - {"encode_page_control", encode_rfc2696, METH_VARARGS}, - {"decode_page_control", decode_rfc2696, METH_VARARGS}, - {"encode_valuesreturnfilter_control", encode_rfc3876, METH_VARARGS}, - {"encode_assertion_control", encode_assertion_control, METH_VARARGS}, - {NULL, NULL} -}; - -void -LDAPinit_control(PyObject *d) -{ - LDAPadd_methods(d, methods); -} diff --git a/Modules/ldapmodule.c b/Modules/ldapmodule.c index cb3f58fb..552422e0 100644 --- a/Modules/ldapmodule.c +++ b/Modules/ldapmodule.c @@ -9,58 +9,55 @@ static char version_str[] = STR(LDAPMODULE_VERSION); static char author_str[] = STR(LDAPMODULE_AUTHOR); static char license_str[] = STR(LDAPMODULE_LICENSE); -static void +static int init_pkginfo(PyObject *m) { - PyModule_AddStringConstant(m, "__version__", version_str); - PyModule_AddStringConstant(m, "__author__", author_str); - PyModule_AddStringConstant(m, "__license__", license_str); + if (PyModule_AddStringConstant(m, "__version__", version_str) != 0) + return -1; + if (PyModule_AddStringConstant(m, "__author__", author_str) != 0) + return -1; + if (PyModule_AddStringConstant(m, "__license__", license_str) != 0) + return -1; + return 0; } -/* dummy module methods */ -static PyMethodDef methods[] = { +static PyMethodDef ldap_functions[] = { + // functions.c + {"initialize", LDAPMod_initialize, METH_VARARGS}, +#ifdef HAVE_LDAP_INIT_FD + {"initialize_fd", LDAPMod_initialize_fd, METH_VARARGS}, +#endif + {"str2dn", LDAPMod_str2dn, METH_VARARGS}, + {"set_option", LDAPMod_set_option, METH_VARARGS}, + {"get_option", LDAPMod_get_option, METH_VARARGS}, + // ldapcontrol.c + {"encode_page_control", LDAPMod_encode_rfc2696, METH_VARARGS}, + {"decode_page_control", LDAPMod_decode_rfc2696, METH_VARARGS}, + {"encode_valuesreturnfilter_control", LDAPMod_encode_rfc3876, + METH_VARARGS}, + {"encode_assertion_control", LDAPMod_encode_assertion_control, + METH_VARARGS}, {NULL, NULL} }; +/* module initialisation */ +static PyModuleDef_Slot ldap_slots[] = { + {Py_mod_exec, LDAPMod_init_type}, + {Py_mod_exec, LDAPMod_init_constants}, + {Py_mod_exec, init_pkginfo}, + {0, NULL} +}; + static struct PyModuleDef ldap_moduledef = { PyModuleDef_HEAD_INIT, - "_ldap", /* m_name */ - "", /* m_doc */ - -1, /* m_size */ - methods, /* m_methods */ + .m_name = "_ldap", + .m_size = 0, + .m_methods = ldap_functions, + .m_slots = ldap_slots, }; -/* module initialisation */ - PyMODINIT_FUNC PyInit__ldap() { - PyObject *m, *d; - - /* Create the module and add the functions */ - m = PyModule_Create(&ldap_moduledef); - - /* Initialize LDAP class */ - if (PyType_Ready(&LDAP_Type) < 0) { - Py_DECREF(m); - return NULL; - } - - /* Add some symbolic constants to the module */ - d = PyModule_GetDict(m); - - init_pkginfo(m); - - if (LDAPinit_constants(m) == -1) { - return NULL; - } - - LDAPinit_functions(d); - LDAPinit_control(d); - - /* Check for errors */ - if (PyErr_Occurred()) - Py_FatalError("can't initialize module _ldap"); - - return m; + return PyModuleDef_Init(&ldap_moduledef); } diff --git a/Modules/pythonldap.h b/Modules/pythonldap.h index 7703af5e..5d42cd27 100644 --- a/Modules/pythonldap.h +++ b/Modules/pythonldap.h @@ -55,8 +55,6 @@ LDAP_F(int) ldap_init_fd(ber_socket_t fd, int proto, LDAP_CONST char *url, PYLDAP_FUNC(PyObject *) LDAPerror_TypeError(const char *, PyObject *); -PYLDAP_FUNC(void) LDAPadd_methods(PyObject *d, PyMethodDef *methods); - #define PyNone_Check(o) ((o) == Py_None) /* *** berval *** */ @@ -64,7 +62,7 @@ PYLDAP_FUNC(PyObject *) LDAPberval_to_object(const struct berval *bv); PYLDAP_FUNC(PyObject *) LDAPberval_to_unicode_object(const struct berval *bv); /* *** constants *** */ -PYLDAP_FUNC(int) LDAPinit_constants(PyObject *m); +PYLDAP_FUNC(int) LDAPMod_init_constants(PyObject *m); PYLDAP_DATA(PyObject *) LDAPexception_class; PYLDAP_FUNC(PyObject *) LDAPerror(LDAP *); @@ -79,14 +77,14 @@ PYLDAP_FUNC(PyObject *) LDAPerr(int errnum); #define LDAP_CONTROL_VALUESRETURNFILTER "1.2.826.0.1.3344810.2.3" /* RFC 3876 */ #endif /* !LDAP_CONTROL_VALUESRETURNFILTER */ -/* *** functions *** */ -PYLDAP_FUNC(void) LDAPinit_functions(PyObject *); - /* *** ldapcontrol *** */ -PYLDAP_FUNC(void) LDAPinit_control(PyObject *d); PYLDAP_FUNC(void) LDAPControl_List_DEL(LDAPControl **); PYLDAP_FUNC(int) LDAPControls_from_object(PyObject *, LDAPControl ***); PYLDAP_FUNC(PyObject *) LDAPControls_to_List(LDAPControl **ldcs); +PYLDAP_FUNC(PyObject *) LDAPMod_encode_rfc2696(PyObject *, PyObject *); +PYLDAP_FUNC(PyObject *) LDAPMod_decode_rfc2696(PyObject *, PyObject *); +PYLDAP_FUNC(PyObject *) LDAPMod_encode_rfc3876(PyObject *, PyObject *); +PYLDAP_FUNC(PyObject *) LDAPMod_encode_assertion_control(PyObject *, PyObject *); /* *** ldapobject *** */ typedef struct { @@ -97,6 +95,7 @@ typedef struct { PYLDAP_DATA(PyTypeObject) LDAP_Type; PYLDAP_FUNC(LDAPObject *) newLDAPObject(LDAP *); +PYLDAP_FUNC(int) LDAPMod_init_type(PyObject *module); /* macros to allow thread saving in the context of an LDAP connection */ @@ -128,4 +127,13 @@ PYLDAP_FUNC(int) LDAP_set_option(LDAPObject *self, int option, PYLDAP_FUNC(PyObject *) LDAP_get_option(LDAPObject *self, int option); PYLDAP_FUNC(void) set_timeval_from_double(struct timeval *tv, double d); +/* *** functions *** */ +PYLDAP_FUNC(PyObject *) LDAPMod_initialize(PyObject *, PyObject *); +#ifdef HAVE_LDAP_INIT_FD +PYLDAP_FUNC(PyObject *) LDAPMod_initialize_fd(PyObject *, PyObject *); +#endif +PYLDAP_FUNC(PyObject *) LDAPMod_str2dn(PyObject *, PyObject *); +PYLDAP_FUNC(PyObject *) LDAPMod_set_option(PyObject *, PyObject *); +PYLDAP_FUNC(PyObject *) LDAPMod_get_option(PyObject *, PyObject *); + #endif /* pythonldap_h */