From 66c06132cefb72b22b2c6ac50688a6d8209d4a3e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 22 May 2022 21:26:23 +0200 Subject: [PATCH 1/3] gh-90763: Modernise xx template module initialisation Use C APIs such as PyModule_AddType instead of PyModule_AddObject. Also remove incorrect module decrefs if module fails to initialise. --- Modules/xxlimited_35.c | 74 +++++++++++++++++++++++------------------- Modules/xxmodule.c | 39 +++++++++++----------- 2 files changed, 60 insertions(+), 53 deletions(-) diff --git a/Modules/xxlimited_35.c b/Modules/xxlimited_35.c index 647abf6721276c..84b37f6f862f5a 100644 --- a/Modules/xxlimited_35.c +++ b/Modules/xxlimited_35.c @@ -124,7 +124,7 @@ static PyType_Slot Xxo_Type_slots[] = { }; static PyType_Spec Xxo_Type_spec = { - "xxlimited.Xxo", + "xxlimited_35.Xxo", sizeof(XxoObject), 0, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, @@ -189,7 +189,7 @@ static PyType_Slot Str_Type_slots[] = { }; static PyType_Spec Str_Type_spec = { - "xxlimited.Str", + "xxlimited_35.Str", 0, 0, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, @@ -212,7 +212,7 @@ static PyType_Slot Null_Type_slots[] = { }; static PyType_Spec Null_Type_spec = { - "xxlimited.Null", + "xxlimited_35.Null", 0, /* basicsize */ 0, /* itemsize */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, @@ -236,11 +236,24 @@ static PyMethodDef xx_methods[] = { PyDoc_STRVAR(module_doc, "This is a module for testing limited API from Python 3.5."); +static PyObject * +create_and_add_type(PyObject *module, const char *name, PyType_Spec *spec) +{ + PyObject *type = PyType_FromSpec(spec); + if (type == NULL) { + return NULL; + } + int rc = PyModule_AddObject(module, name, type); + if (rc < 0) { + Py_DECREF(type); + return NULL; + } + return type; +} + static int xx_modexec(PyObject *m) { - PyObject *o; - /* Due to cross platform compiler issues the slots must be filled * here. It's required for portability to Windows without requiring * C++. */ @@ -248,40 +261,33 @@ xx_modexec(PyObject *m) Null_Type_slots[1].pfunc = PyType_GenericNew; Str_Type_slots[0].pfunc = &PyUnicode_Type; - Xxo_Type = PyType_FromSpec(&Xxo_Type_spec); - if (Xxo_Type == NULL) - goto fail; - /* Add some symbolic constants to the module */ if (ErrorObject == NULL) { - ErrorObject = PyErr_NewException("xxlimited.error", NULL, NULL); - if (ErrorObject == NULL) - goto fail; + ErrorObject = PyErr_NewException("xxlimited_35.error", NULL, NULL); + if (ErrorObject == NULL) { + return -1; + } } Py_INCREF(ErrorObject); - PyModule_AddObject(m, "error", ErrorObject); - - /* Add Xxo */ - o = PyType_FromSpec(&Xxo_Type_spec); - if (o == NULL) - goto fail; - PyModule_AddObject(m, "Xxo", o); - - /* Add Str */ - o = PyType_FromSpec(&Str_Type_spec); - if (o == NULL) - goto fail; - PyModule_AddObject(m, "Str", o); - - /* Add Null */ - o = PyType_FromSpec(&Null_Type_spec); - if (o == NULL) - goto fail; - PyModule_AddObject(m, "Null", o); + int rc = PyModule_AddObject(m, "error", ErrorObject); + if (rc < 0) { + Py_DECREF(ErrorObject); + return -1; + } + + /* Add Xxo, Str, and Null types */ + Xxo_Type = create_and_add_type(m, "Xxo", &Xxo_Type_spec); + if (Xxo_Type == NULL) { + return -1; + } + if (create_and_add_type(m, "Str", &Str_Type_spec) == NULL) { + return -1; + } + if (create_and_add_type(m, "Null", &Null_Type_spec) == NULL) { + return -1; + } + return 0; - fail: - Py_XDECREF(m); - return -1; } diff --git a/Modules/xxmodule.c b/Modules/xxmodule.c index edcd62157c02f3..a6e5071d1d6303 100644 --- a/Modules/xxmodule.c +++ b/Modules/xxmodule.c @@ -358,31 +358,32 @@ xx_exec(PyObject *m) /* Finalize the type object including setting type of the new type * object; doing it here is required for portability, too. */ - if (PyType_Ready(&Xxo_Type) < 0) - goto fail; + if (PyType_Ready(&Xxo_Type) < 0) { + return -1; + } /* Add some symbolic constants to the module */ if (ErrorObject == NULL) { ErrorObject = PyErr_NewException("xx.error", NULL, NULL); - if (ErrorObject == NULL) - goto fail; + if (ErrorObject == NULL) { + return -1; + } + } + int rc = PyModule_AddType(m, (PyTypeObject *)ErrorObject); + Py_DECREF(ErrorObject); + if (rc < 0) { + return -1; } - Py_INCREF(ErrorObject); - PyModule_AddObject(m, "error", ErrorObject); - - /* Add Str */ - if (PyType_Ready(&Str_Type) < 0) - goto fail; - PyModule_AddObject(m, "Str", (PyObject *)&Str_Type); - - /* Add Null */ - if (PyType_Ready(&Null_Type) < 0) - goto fail; - PyModule_AddObject(m, "Null", (PyObject *)&Null_Type); + + /* Add Str and Null types */ + if (PyModule_AddType(m, &Str_Type) < 0) { + return -1; + } + if (PyModule_AddType(m, &Null_Type) < 0) { + return -1; + } + return 0; - fail: - Py_XDECREF(m); - return -1; } static struct PyModuleDef_Slot xx_slots[] = { From f0fb621dc139c142a1b1eb47606b737a85888486 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 23 May 2022 07:24:09 +0200 Subject: [PATCH 2/3] Address review: remove unneeded rc variables --- Modules/xxlimited_35.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Modules/xxlimited_35.c b/Modules/xxlimited_35.c index 84b37f6f862f5a..918918af6a2643 100644 --- a/Modules/xxlimited_35.c +++ b/Modules/xxlimited_35.c @@ -243,8 +243,7 @@ create_and_add_type(PyObject *module, const char *name, PyType_Spec *spec) if (type == NULL) { return NULL; } - int rc = PyModule_AddObject(module, name, type); - if (rc < 0) { + if (PyModule_AddObject(module, name, type) < 0) { Py_DECREF(type); return NULL; } @@ -269,8 +268,7 @@ xx_modexec(PyObject *m) } } Py_INCREF(ErrorObject); - int rc = PyModule_AddObject(m, "error", ErrorObject); - if (rc < 0) { + if (PyModule_AddObject(m, "error", ErrorObject) < 0) { Py_DECREF(ErrorObject); return -1; } From 5ce42f783079f54891f4a564a98c9213b03cc462 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 23 May 2022 13:52:15 +0200 Subject: [PATCH 3/3] Address review: remove helper from xxlimited_35 --- Modules/xxlimited_35.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/Modules/xxlimited_35.c b/Modules/xxlimited_35.c index 918918af6a2643..8d29c71951768a 100644 --- a/Modules/xxlimited_35.c +++ b/Modules/xxlimited_35.c @@ -236,23 +236,11 @@ static PyMethodDef xx_methods[] = { PyDoc_STRVAR(module_doc, "This is a module for testing limited API from Python 3.5."); -static PyObject * -create_and_add_type(PyObject *module, const char *name, PyType_Spec *spec) -{ - PyObject *type = PyType_FromSpec(spec); - if (type == NULL) { - return NULL; - } - if (PyModule_AddObject(module, name, type) < 0) { - Py_DECREF(type); - return NULL; - } - return type; -} - static int xx_modexec(PyObject *m) { + PyObject *o; + /* Due to cross platform compiler issues the slots must be filled * here. It's required for portability to Windows without requiring * C++. */ @@ -273,15 +261,33 @@ xx_modexec(PyObject *m) return -1; } - /* Add Xxo, Str, and Null types */ - Xxo_Type = create_and_add_type(m, "Xxo", &Xxo_Type_spec); + /* Add Xxo */ + Xxo_Type = PyType_FromSpec(&Xxo_Type_spec); if (Xxo_Type == NULL) { return -1; } - if (create_and_add_type(m, "Str", &Str_Type_spec) == NULL) { + if (PyModule_AddObject(m, "Xxo", Xxo_Type) < 0) { + Py_DECREF(Xxo_Type); + return -1; + } + + /* Add Str */ + o = PyType_FromSpec(&Str_Type_spec); + if (o == NULL) { + return -1; + } + if (PyModule_AddObject(m, "Str", o) < 0) { + Py_DECREF(o); + return -1; + } + + /* Add Null */ + o = PyType_FromSpec(&Null_Type_spec); + if (o == NULL) { return -1; } - if (create_and_add_type(m, "Null", &Null_Type_spec) == NULL) { + if (PyModule_AddObject(m, "Null", o) < 0) { + Py_DECREF(o); return -1; }