From e48b24d0da750fa24fedd06232ecb6e8f415f201 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Thu, 1 Oct 2020 21:17:04 -0500 Subject: [PATCH 1/8] error handling 1 --- Modules/pyexpat.c | 238 ++++++++++++++++++++++++++++++---------------- 1 file changed, 155 insertions(+), 83 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 12ae66d945bda8..9c163415a8b950 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1632,100 +1632,131 @@ static int init_handler_descrs(void) } PyMODINIT_FUNC -MODULE_INITFUNC(void) +PyInit_pyexpat(void) { - PyObject *m, *d; - PyObject *errmod_name = PyUnicode_FromString(MODULE_NAME ".errors"); - PyObject *errors_module; - PyObject *modelmod_name; - PyObject *model_module; - PyObject *tmpnum, *tmpstr; - PyObject *codes_dict; - PyObject *rev_codes_dict; - int res; - static struct PyExpat_CAPI capi; - PyObject *capi_object; - - if (errmod_name == NULL) - return NULL; - modelmod_name = PyUnicode_FromString(MODULE_NAME ".model"); - if (modelmod_name == NULL) + PyObject *mod = PyModule_Create(&pyexpatmodule); + if (mod == NULL) { return NULL; + } - if (PyType_Ready(&Xmlparsetype) < 0 || init_handler_descrs() < 0) - return NULL; + PyObject *codes_dict = NULL, *rev_codes_dict = NULL; - /* Create the module and add the functions */ - m = PyModule_Create(&pyexpatmodule); - if (m == NULL) - return NULL; + if (PyType_Ready(&Xmlparsetype) < 0) { + goto error; + } + + if (init_handler_descrs() < 0) { + goto error; + } /* Add some symbolic constants to the module */ if (ErrorObject == NULL) { ErrorObject = PyErr_NewException("xml.parsers.expat.ExpatError", - NULL, NULL); - if (ErrorObject == NULL) - return NULL; + NULL, NULL); } + if (ErrorObject == NULL) { + goto error; + } + Py_INCREF(ErrorObject); - PyModule_AddObject(m, "error", ErrorObject); + if (PyModule_AddObject(mod, "error", ErrorObject) < 0) { + goto error; + } Py_INCREF(ErrorObject); - PyModule_AddObject(m, "ExpatError", ErrorObject); + if (PyModule_AddObject(mod, "ExpatError", ErrorObject) < 0) { + goto error; + } Py_INCREF(&Xmlparsetype); - PyModule_AddObject(m, "XMLParserType", (PyObject *) &Xmlparsetype); + if (PyModule_AddObject(mod, "XMLParserType", + (PyObject *) &Xmlparsetype) < 0) { + goto error; + } - PyModule_AddStringConstant(m, "EXPAT_VERSION", - XML_ExpatVersion()); + if (PyModule_AddStringConstant(mod, "EXPAT_VERSION", + XML_ExpatVersion()) < 0) { + goto error; + } { XML_Expat_Version info = XML_ExpatVersionInfo(); - PyModule_AddObject(m, "version_info", - Py_BuildValue("(iii)", info.major, - info.minor, info.micro)); + if (PyModule_AddObject(mod, "version_info", Py_BuildValue("(iii)", + info.major, info.minor, info.micro)) < 0) { + goto error; + } } /* XXX When Expat supports some way of figuring out how it was compiled, this should check and set native_encoding appropriately. */ - PyModule_AddStringConstant(m, "native_encoding", "UTF-8"); + if (PyModule_AddStringConstant(mod, "native_encoding", "UTF-8") < 0) { + goto error; + } - d = PyModule_GetDict(m); + PyObject *d = PyModule_GetDict(mod); if (d == NULL) { - Py_DECREF(m); - return NULL; + goto error; } - errors_module = PyDict_GetItemWithError(d, errmod_name); + + PyObject *errmod_name = PyUnicode_FromString(MODULE_NAME ".errors"); + if (errmod_name == NULL) { + goto error; + } + + PyObject *errors_module = PyDict_GetItemWithError(d, errmod_name); if (errors_module == NULL && !PyErr_Occurred()) { errors_module = PyModule_New(MODULE_NAME ".errors"); if (errors_module != NULL) { - _PyImport_SetModule(errmod_name, errors_module); + if (_PyImport_SetModule(errmod_name, errors_module) < 0) { + Py_DECREF(errors_module); + Py_CLEAR(errmod_name); + goto error; + } /* gives away the reference to errors_module */ - PyModule_AddObject(m, "errors", errors_module); + if (PyModule_AddObject(mod, "errors", errors_module) < 0) { + Py_DECREF(errors_module); + Py_CLEAR(errmod_name); + goto error; + } } } - Py_DECREF(errmod_name); - model_module = PyDict_GetItemWithError(d, modelmod_name); + Py_CLEAR(errmod_name); + + PyObject *modelmod_name = PyUnicode_FromString(MODULE_NAME ".model"); + if (modelmod_name == NULL) { + goto error; + } + + PyObject *model_module = PyDict_GetItemWithError(d, modelmod_name); if (model_module == NULL && !PyErr_Occurred()) { model_module = PyModule_New(MODULE_NAME ".model"); if (model_module != NULL) { - _PyImport_SetModule(modelmod_name, model_module); + if (_PyImport_SetModule(modelmod_name, model_module) < 0) { + Py_DECREF(model_module); + Py_CLEAR(modelmod_name); + goto error; + } /* gives away the reference to model_module */ - PyModule_AddObject(m, "model", model_module); + if (PyModule_AddObject(mod, "model", model_module) < 0) { + Py_DECREF(model_module); + Py_CLEAR(modelmod_name); + goto error; + } } } - Py_DECREF(modelmod_name); + Py_CLEAR(modelmod_name); + if (errors_module == NULL || model_module == NULL) { /* Don't core dump later! */ - Py_DECREF(m); - return NULL; + goto error; } #if XML_COMBINED_VERSION > 19505 { const XML_Feature *features = XML_GetFeatureList(); PyObject *list = PyList_New(0); - if (list == NULL) + if (list == NULL) { /* just ignore it */ PyErr_Clear(); + } else { int i = 0; for (; features[i].feature != XML_FEATURE_END; ++i) { @@ -1744,8 +1775,9 @@ MODULE_INITFUNC(void) break; } } - if (list != NULL) - PyModule_AddObject(m, "features", list); + if (list != NULL) { + PyModule_AddObject(mod, "features", list); + } } } #endif @@ -1753,26 +1785,39 @@ MODULE_INITFUNC(void) codes_dict = PyDict_New(); rev_codes_dict = PyDict_New(); if (codes_dict == NULL || rev_codes_dict == NULL) { - Py_XDECREF(codes_dict); - Py_XDECREF(rev_codes_dict); - return NULL; + goto error; } -#define MYCONST(name) \ - if (PyModule_AddStringConstant(errors_module, #name, \ - XML_ErrorString(name)) < 0) \ - return NULL; \ - tmpnum = PyLong_FromLong(name); \ - if (tmpnum == NULL) return NULL; \ - res = PyDict_SetItemString(codes_dict, \ - XML_ErrorString(name), tmpnum); \ - if (res < 0) return NULL; \ - tmpstr = PyUnicode_FromString(XML_ErrorString(name)); \ - if (tmpstr == NULL) return NULL; \ - res = PyDict_SetItem(rev_codes_dict, tmpnum, tmpstr); \ - Py_DECREF(tmpstr); \ - Py_DECREF(tmpnum); \ - if (res < 0) return NULL; \ + int res; + PyObject *tmpnum, *tmpstr; +#define MYCONST(name) do { \ + if (PyModule_AddStringConstant(errors_module, #name, \ + XML_ErrorString(name)) < 0) { \ + goto error; \ + } \ + tmpnum = PyLong_FromLong(name); \ + if (tmpnum == NULL) { \ + goto error; \ + } \ + res = PyDict_SetItemString(codes_dict, \ + XML_ErrorString(name), tmpnum); \ + if (res < 0) { \ + Py_DECREF(tmpnum); \ + goto error; \ + } \ + tmpstr = PyUnicode_FromString(XML_ErrorString(name)); \ + if (tmpstr == NULL) { \ + Py_DECREF(tmpnum); \ + goto error; \ + } \ + res = PyDict_SetItem(rev_codes_dict, tmpnum, tmpstr); \ + Py_CLEAR(tmpstr); \ + Py_CLEAR(tmpnum); \ + if (res < 0) { \ + goto error; \ + } \ + } while(0); + MYCONST(XML_ERROR_NO_MEMORY); MYCONST(XML_ERROR_SYNTAX); @@ -1816,25 +1861,39 @@ MODULE_INITFUNC(void) if (PyModule_AddStringConstant(errors_module, "__doc__", "Constants used to describe " - "error conditions.") < 0) - return NULL; + "error conditions.") < 0) { + goto error; + } - if (PyModule_AddObject(errors_module, "codes", codes_dict) < 0) - return NULL; - if (PyModule_AddObject(errors_module, "messages", rev_codes_dict) < 0) - return NULL; + if (PyModule_AddObject(errors_module, "codes", codes_dict) < 0) { + goto error; + } + if (PyModule_AddObject(errors_module, "messages", rev_codes_dict) < 0) { + goto error; + } #undef MYCONST -#define MYCONST(c) PyModule_AddIntConstant(m, #c, c) +#define MYCONST(c) do { \ + if (PyModule_AddIntConstant(mod, #c, c) < 0) { \ + goto error; \ + } \ + } while(0); MYCONST(XML_PARAM_ENTITY_PARSING_NEVER); MYCONST(XML_PARAM_ENTITY_PARSING_UNLESS_STANDALONE); MYCONST(XML_PARAM_ENTITY_PARSING_ALWAYS); #undef MYCONST -#define MYCONST(c) PyModule_AddIntConstant(model_module, #c, c) - PyModule_AddStringConstant(model_module, "__doc__", - "Constants used to interpret content model information."); +#define MYCONST(c) do { \ + if (PyModule_AddIntConstant(model_module, #c, c) < 0) { \ + goto error; \ + } \ + } while(0); + if (PyModule_AddStringConstant( + model_module, "__doc__", + "Constants used to interpret content model information.") < 0) { + goto error; + } MYCONST(XML_CTYPE_EMPTY); MYCONST(XML_CTYPE_ANY); @@ -1849,6 +1908,7 @@ MODULE_INITFUNC(void) MYCONST(XML_CQUANT_PLUS); #undef MYCONST + static struct PyExpat_CAPI capi; /* initialize pyexpat dispatch table */ capi.size = sizeof(capi); capi.magic = PyExpat_CAPI_MAGIC; @@ -1880,10 +1940,22 @@ MODULE_INITFUNC(void) #endif /* export using capsule */ - capi_object = PyCapsule_New(&capi, PyExpat_CAPSULE_NAME, NULL); - if (capi_object) - PyModule_AddObject(m, "expat_CAPI", capi_object); - return m; + PyObject *capi_object = PyCapsule_New(&capi, PyExpat_CAPSULE_NAME, NULL); + if (capi_object == NULL) { + goto error; + } + + if (PyModule_AddObject(mod, "expat_CAPI", capi_object) < 0) { + Py_DECREF(capi_object); + goto error; + } + + return mod; + +error: + Py_CLEAR(codes_dict); + Py_CLEAR(rev_codes_dict); + return NULL; } static void From 0e536f62205e2f4ae3fe4858d7a18994f7bf9d2d Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Thu, 1 Oct 2020 21:47:26 -0500 Subject: [PATCH 2/8] address PR comments --- Modules/pyexpat.c | 98 +++++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 51 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 9c163415a8b950..fb9a7710626657 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1660,26 +1660,33 @@ PyInit_pyexpat(void) Py_INCREF(ErrorObject); if (PyModule_AddObject(mod, "error", ErrorObject) < 0) { + Py_DECREF(ErrorObject); goto error; } Py_INCREF(ErrorObject); if (PyModule_AddObject(mod, "ExpatError", ErrorObject) < 0) { + Py_DECREF(ErrorObject); goto error; } Py_INCREF(&Xmlparsetype); if (PyModule_AddObject(mod, "XMLParserType", (PyObject *) &Xmlparsetype) < 0) { + Py_DECREF(&Xmlparsetype); goto error; } if (PyModule_AddStringConstant(mod, "EXPAT_VERSION", - XML_ExpatVersion()) < 0) { + XML_ExpatVersion()) < 0) { goto error; } { XML_Expat_Version info = XML_ExpatVersionInfo(); - if (PyModule_AddObject(mod, "version_info", Py_BuildValue("(iii)", - info.major, info.minor, info.micro)) < 0) { + PyObject *versionInfo = Py_BuildValue("(iii)", + info.major, + info.minor, + info.micro); + if (PyModule_AddObject(mod, "version_info", versionInfo) < 0) { + Py_DECREF(versionInfo); goto error; } } @@ -1691,8 +1698,8 @@ PyInit_pyexpat(void) goto error; } - PyObject *d = PyModule_GetDict(mod); - if (d == NULL) { + PyObject *errors_module = PyModule_New(MODULE_NAME ".errors"); + if (errors_module == NULL) { goto error; } @@ -1701,46 +1708,39 @@ PyInit_pyexpat(void) goto error; } - PyObject *errors_module = PyDict_GetItemWithError(d, errmod_name); - if (errors_module == NULL && !PyErr_Occurred()) { - errors_module = PyModule_New(MODULE_NAME ".errors"); - if (errors_module != NULL) { - if (_PyImport_SetModule(errmod_name, errors_module) < 0) { - Py_DECREF(errors_module); - Py_CLEAR(errmod_name); - goto error; - } - /* gives away the reference to errors_module */ - if (PyModule_AddObject(mod, "errors", errors_module) < 0) { - Py_DECREF(errors_module); - Py_CLEAR(errmod_name); - goto error; - } - } + if (_PyImport_SetModule(errmod_name, errors_module) < 0) { + Py_DECREF(errors_module); + Py_CLEAR(errmod_name); + goto error; + } + /* gives away the reference to errors_module */ + if (PyModule_AddObject(mod, "errors", errors_module) < 0) { + Py_DECREF(errors_module); + Py_CLEAR(errmod_name); + goto error; } Py_CLEAR(errmod_name); + PyObject *model_module = PyModule_New(MODULE_NAME ".model"); + if (model_module == NULL) { + goto error; + } + PyObject *modelmod_name = PyUnicode_FromString(MODULE_NAME ".model"); if (modelmod_name == NULL) { goto error; } - PyObject *model_module = PyDict_GetItemWithError(d, modelmod_name); - if (model_module == NULL && !PyErr_Occurred()) { - model_module = PyModule_New(MODULE_NAME ".model"); - if (model_module != NULL) { - if (_PyImport_SetModule(modelmod_name, model_module) < 0) { - Py_DECREF(model_module); - Py_CLEAR(modelmod_name); - goto error; - } - /* gives away the reference to model_module */ - if (PyModule_AddObject(mod, "model", model_module) < 0) { - Py_DECREF(model_module); - Py_CLEAR(modelmod_name); - goto error; - } - } + if (_PyImport_SetModule(modelmod_name, model_module) < 0) { + Py_DECREF(model_module); + Py_CLEAR(modelmod_name); + goto error; + } + /* gives away the reference to model_module */ + if (PyModule_AddObject(mod, "model", model_module) < 0) { + Py_DECREF(model_module); + Py_CLEAR(modelmod_name); + goto error; } Py_CLEAR(modelmod_name); @@ -1754,30 +1754,26 @@ PyInit_pyexpat(void) const XML_Feature *features = XML_GetFeatureList(); PyObject *list = PyList_New(0); if (list == NULL) { - /* just ignore it */ - PyErr_Clear(); + goto error; } else { int i = 0; for (; features[i].feature != XML_FEATURE_END; ++i) { - int ok; PyObject *item = Py_BuildValue("si", features[i].name, features[i].value); if (item == NULL) { Py_DECREF(list); - list = NULL; - break; + goto error; } - ok = PyList_Append(list, item); + int ok = PyList_Append(list, item); Py_DECREF(item); if (ok < 0) { PyErr_Clear(); break; } } - if (list != NULL) { - PyModule_AddObject(mod, "features", list); - } + + PyModule_AddObject(mod, "features", list); } } #endif @@ -1792,7 +1788,7 @@ PyInit_pyexpat(void) PyObject *tmpnum, *tmpstr; #define MYCONST(name) do { \ if (PyModule_AddStringConstant(errors_module, #name, \ - XML_ErrorString(name)) < 0) { \ + XML_ErrorString(name)) < 0) {\ goto error; \ } \ tmpnum = PyLong_FromLong(name); \ @@ -1800,7 +1796,7 @@ PyInit_pyexpat(void) goto error; \ } \ res = PyDict_SetItemString(codes_dict, \ - XML_ErrorString(name), tmpnum); \ + XML_ErrorString(name), tmpnum); \ if (res < 0) { \ Py_DECREF(tmpnum); \ goto error; \ @@ -1816,7 +1812,7 @@ PyInit_pyexpat(void) if (res < 0) { \ goto error; \ } \ - } while(0); + } while(0) MYCONST(XML_ERROR_NO_MEMORY); @@ -1878,7 +1874,7 @@ PyInit_pyexpat(void) if (PyModule_AddIntConstant(mod, #c, c) < 0) { \ goto error; \ } \ - } while(0); + } while(0) MYCONST(XML_PARAM_ENTITY_PARSING_NEVER); MYCONST(XML_PARAM_ENTITY_PARSING_UNLESS_STANDALONE); MYCONST(XML_PARAM_ENTITY_PARSING_ALWAYS); @@ -1888,7 +1884,7 @@ PyInit_pyexpat(void) if (PyModule_AddIntConstant(model_module, #c, c) < 0) { \ goto error; \ } \ - } while(0); + } while(0) if (PyModule_AddStringConstant( model_module, "__doc__", "Constants used to interpret content model information.") < 0) { From 0b5651088bf99cf3e4aee89ec5379ecfc9f038b4 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Sat, 10 Oct 2020 13:13:03 -0500 Subject: [PATCH 3/8] update based on PR review --- Modules/pyexpat.c | 89 +++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index fb9a7710626657..932a8acc0b5c1c 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1587,18 +1587,6 @@ PyDoc_STRVAR(pyexpat_module_documentation, #define MODULE_INITFUNC PyInit_pyexpat #endif -static struct PyModuleDef pyexpatmodule = { - PyModuleDef_HEAD_INIT, - MODULE_NAME, - pyexpat_module_documentation, - -1, - pyexpat_methods, - NULL, - NULL, - NULL, - NULL -}; - static int init_handler_descrs(void) { int i; @@ -1631,14 +1619,9 @@ static int init_handler_descrs(void) return 0; } -PyMODINIT_FUNC -PyInit_pyexpat(void) +static int +pyexpat_exec(PyObject *mod) { - PyObject *mod = PyModule_Create(&pyexpatmodule); - if (mod == NULL) { - return NULL; - } - PyObject *codes_dict = NULL, *rev_codes_dict = NULL; if (PyType_Ready(&Xmlparsetype) < 0) { @@ -1652,7 +1635,7 @@ PyInit_pyexpat(void) /* Add some symbolic constants to the module */ if (ErrorObject == NULL) { ErrorObject = PyErr_NewException("xml.parsers.expat.ExpatError", - NULL, NULL); + NULL, NULL); } if (ErrorObject == NULL) { goto error; @@ -1756,24 +1739,26 @@ PyInit_pyexpat(void) if (list == NULL) { goto error; } - else { - int i = 0; - for (; features[i].feature != XML_FEATURE_END; ++i) { - PyObject *item = Py_BuildValue("si", features[i].name, - features[i].value); - if (item == NULL) { - Py_DECREF(list); - goto error; - } - int ok = PyList_Append(list, item); - Py_DECREF(item); - if (ok < 0) { - PyErr_Clear(); - break; - } - } - PyModule_AddObject(mod, "features", list); + int i = 0; + for (; features[i].feature != XML_FEATURE_END; ++i) { + PyObject *item = Py_BuildValue("si", features[i].name, + features[i].value); + if (item == NULL) { + Py_DECREF(list); + goto error; + } + int ok = PyList_Append(list, item); + Py_DECREF(item); + if (ok < 0) { + PyErr_Clear(); + Py_DECREF(list); + goto error; + } + } + if (PyModule_AddObject(mod, "features", list) < 0) { + Py_DECREF(list); + goto error; } } #endif @@ -1875,6 +1860,7 @@ PyInit_pyexpat(void) goto error; \ } \ } while(0) + MYCONST(XML_PARAM_ENTITY_PARSING_NEVER); MYCONST(XML_PARAM_ENTITY_PARSING_UNLESS_STANDALONE); MYCONST(XML_PARAM_ENTITY_PARSING_ALWAYS); @@ -1885,6 +1871,7 @@ PyInit_pyexpat(void) goto error; \ } \ } while(0) + if (PyModule_AddStringConstant( model_module, "__doc__", "Constants used to interpret content model information.") < 0) { @@ -1946,12 +1933,32 @@ PyInit_pyexpat(void) goto error; } - return mod; + return 0; error: - Py_CLEAR(codes_dict); - Py_CLEAR(rev_codes_dict); - return NULL; + Py_XDECREF(codes_dict); + Py_XDECREF(rev_codes_dict); + return -1; +} + +static PyModuleDef_Slot pyexpat_slots[] = { + {Py_mod_exec, pyexpat_exec}, + {0, NULL} +}; + +static struct PyModuleDef pyexpatmodule = { + PyModuleDef_HEAD_INIT, + .m_name = MODULE_NAME, + .m_doc = pyexpat_module_documentation, + .m_size = 0, + .m_methods = pyexpat_methods, + .m_slots = pyexpat_slots +}; + +PyMODINIT_FUNC +PyInit_pyexpat(void) +{ + return PyModuleDef_Init(&pyexpatmodule); } static void From a994d8a6262fe5a76ffe2c548168238dc324a2b6 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Mon, 12 Oct 2020 13:09:36 -0500 Subject: [PATCH 4/8] pull submodule into helper function --- Modules/pyexpat.c | 78 +++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 43 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 932a8acc0b5c1c..4befb368ea3585 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1619,6 +1619,39 @@ static int init_handler_descrs(void) return 0; } +static PyObject * +add_submodule(PyObject *mod, const char *fullname) +{ + const char *name = strrchr(fullname, '.') + 1; + + PyObject *submodule = PyModule_New(fullname); + if (submodule == NULL) { + return NULL; + } + + PyObject *mod_name = PyUnicode_FromString(fullname); + if (mod_name == NULL) { + Py_DECREF(submodule); + return NULL; + } + + if (_PyImport_SetModule(mod_name, submodule) < 0) { + Py_DECREF(submodule); + Py_DECREF(mod_name); + return NULL; + } + + /* gives away the reference to the submodule */ + if (PyModule_AddObject(mod, name, submodule) < 0) { + Py_DECREF(submodule); + Py_DECREF(mod_name); + return NULL; + } + + Py_DECREF(mod_name); + return submodule; +} + static int pyexpat_exec(PyObject *mod) { @@ -1681,57 +1714,16 @@ pyexpat_exec(PyObject *mod) goto error; } - PyObject *errors_module = PyModule_New(MODULE_NAME ".errors"); + PyObject *errors_module = add_submodule(mod, MODULE_NAME ".errors"); if (errors_module == NULL) { goto error; } - PyObject *errmod_name = PyUnicode_FromString(MODULE_NAME ".errors"); - if (errmod_name == NULL) { - goto error; - } - - if (_PyImport_SetModule(errmod_name, errors_module) < 0) { - Py_DECREF(errors_module); - Py_CLEAR(errmod_name); - goto error; - } - /* gives away the reference to errors_module */ - if (PyModule_AddObject(mod, "errors", errors_module) < 0) { - Py_DECREF(errors_module); - Py_CLEAR(errmod_name); - goto error; - } - Py_CLEAR(errmod_name); - - PyObject *model_module = PyModule_New(MODULE_NAME ".model"); + PyObject *model_module = add_submodule(mod, MODULE_NAME ".model"); if (model_module == NULL) { goto error; } - PyObject *modelmod_name = PyUnicode_FromString(MODULE_NAME ".model"); - if (modelmod_name == NULL) { - goto error; - } - - if (_PyImport_SetModule(modelmod_name, model_module) < 0) { - Py_DECREF(model_module); - Py_CLEAR(modelmod_name); - goto error; - } - /* gives away the reference to model_module */ - if (PyModule_AddObject(mod, "model", model_module) < 0) { - Py_DECREF(model_module); - Py_CLEAR(modelmod_name); - goto error; - } - Py_CLEAR(modelmod_name); - - if (errors_module == NULL || model_module == NULL) { - /* Don't core dump later! */ - goto error; - } - #if XML_COMBINED_VERSION > 19505 { const XML_Feature *features = XML_GetFeatureList(); From fb2bcf7b91966c51daa637f5c067991e9666d8ef Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Sun, 18 Oct 2020 20:34:50 -0500 Subject: [PATCH 5/8] pull submodule code into helper functions --- Modules/pyexpat.c | 252 ++++++++++++++++++++++++---------------------- 1 file changed, 134 insertions(+), 118 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 4befb368ea3585..554376d0530cdf 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1653,110 +1653,15 @@ add_submodule(PyObject *mod, const char *fullname) } static int -pyexpat_exec(PyObject *mod) +add_errors_module(PyObject *mod) { - PyObject *codes_dict = NULL, *rev_codes_dict = NULL; - - if (PyType_Ready(&Xmlparsetype) < 0) { - goto error; - } - - if (init_handler_descrs() < 0) { - goto error; - } - - /* Add some symbolic constants to the module */ - if (ErrorObject == NULL) { - ErrorObject = PyErr_NewException("xml.parsers.expat.ExpatError", - NULL, NULL); - } - if (ErrorObject == NULL) { - goto error; - } - - Py_INCREF(ErrorObject); - if (PyModule_AddObject(mod, "error", ErrorObject) < 0) { - Py_DECREF(ErrorObject); - goto error; - } - Py_INCREF(ErrorObject); - if (PyModule_AddObject(mod, "ExpatError", ErrorObject) < 0) { - Py_DECREF(ErrorObject); - goto error; - } - Py_INCREF(&Xmlparsetype); - if (PyModule_AddObject(mod, "XMLParserType", - (PyObject *) &Xmlparsetype) < 0) { - Py_DECREF(&Xmlparsetype); - goto error; - } - - if (PyModule_AddStringConstant(mod, "EXPAT_VERSION", - XML_ExpatVersion()) < 0) { - goto error; - } - { - XML_Expat_Version info = XML_ExpatVersionInfo(); - PyObject *versionInfo = Py_BuildValue("(iii)", - info.major, - info.minor, - info.micro); - if (PyModule_AddObject(mod, "version_info", versionInfo) < 0) { - Py_DECREF(versionInfo); - goto error; - } - } - /* XXX When Expat supports some way of figuring out how it was - compiled, this should check and set native_encoding - appropriately. - */ - if (PyModule_AddStringConstant(mod, "native_encoding", "UTF-8") < 0) { - goto error; - } - PyObject *errors_module = add_submodule(mod, MODULE_NAME ".errors"); if (errors_module == NULL) { - goto error; - } - - PyObject *model_module = add_submodule(mod, MODULE_NAME ".model"); - if (model_module == NULL) { - goto error; - } - -#if XML_COMBINED_VERSION > 19505 - { - const XML_Feature *features = XML_GetFeatureList(); - PyObject *list = PyList_New(0); - if (list == NULL) { - goto error; - } - - int i = 0; - for (; features[i].feature != XML_FEATURE_END; ++i) { - PyObject *item = Py_BuildValue("si", features[i].name, - features[i].value); - if (item == NULL) { - Py_DECREF(list); - goto error; - } - int ok = PyList_Append(list, item); - Py_DECREF(item); - if (ok < 0) { - PyErr_Clear(); - Py_DECREF(list); - goto error; - } - } - if (PyModule_AddObject(mod, "features", list) < 0) { - Py_DECREF(list); - goto error; - } + return -1; } -#endif - codes_dict = PyDict_New(); - rev_codes_dict = PyDict_New(); + PyObject *codes_dict = PyDict_New(); + PyObject *rev_codes_dict = PyDict_New(); if (codes_dict == NULL || rev_codes_dict == NULL) { goto error; } @@ -1844,30 +1749,34 @@ pyexpat_exec(PyObject *mod) if (PyModule_AddObject(errors_module, "messages", rev_codes_dict) < 0) { goto error; } + #undef MYCONST -#undef MYCONST + return 0; -#define MYCONST(c) do { \ - if (PyModule_AddIntConstant(mod, #c, c) < 0) { \ - goto error; \ - } \ - } while(0) + error: + Py_XDECREF(codes_dict); + Py_XDECREF(rev_codes_dict); + return -1; +} - MYCONST(XML_PARAM_ENTITY_PARSING_NEVER); - MYCONST(XML_PARAM_ENTITY_PARSING_UNLESS_STANDALONE); - MYCONST(XML_PARAM_ENTITY_PARSING_ALWAYS); -#undef MYCONST +static int +add_model_module(PyObject *mod) +{ + PyObject *model_module = add_submodule(mod, MODULE_NAME ".model"); + if (model_module == NULL) { + return -1; + } #define MYCONST(c) do { \ if (PyModule_AddIntConstant(model_module, #c, c) < 0) { \ - goto error; \ + return -1; \ } \ } while(0) if (PyModule_AddStringConstant( model_module, "__doc__", "Constants used to interpret content model information.") < 0) { - goto error; + return -1; } MYCONST(XML_CTYPE_EMPTY); @@ -1882,6 +1791,118 @@ pyexpat_exec(PyObject *mod) MYCONST(XML_CQUANT_REP); MYCONST(XML_CQUANT_PLUS); #undef MYCONST + return 0; +} + +static int +pyexpat_exec(PyObject *mod) +{ + if (PyType_Ready(&Xmlparsetype) < 0) { + return -1; + } + + if (init_handler_descrs() < 0) { + return -1; + } + + /* Add some symbolic constants to the module */ + if (ErrorObject == NULL) { + ErrorObject = PyErr_NewException("xml.parsers.expat.ExpatError", + NULL, NULL); + } + if (ErrorObject == NULL) { + return -1; + } + + Py_INCREF(ErrorObject); + if (PyModule_AddObject(mod, "error", ErrorObject) < 0) { + Py_DECREF(ErrorObject); + return -1; + } + Py_INCREF(ErrorObject); + if (PyModule_AddObject(mod, "ExpatError", ErrorObject) < 0) { + Py_DECREF(ErrorObject); + return -1; + } + Py_INCREF(&Xmlparsetype); + if (PyModule_AddObject(mod, "XMLParserType", + (PyObject *) &Xmlparsetype) < 0) { + Py_DECREF(&Xmlparsetype); + return -1; + } + + if (PyModule_AddStringConstant(mod, "EXPAT_VERSION", + XML_ExpatVersion()) < 0) { + return -1; + } + { + XML_Expat_Version info = XML_ExpatVersionInfo(); + PyObject *versionInfo = Py_BuildValue("(iii)", + info.major, + info.minor, + info.micro); + if (PyModule_AddObject(mod, "version_info", versionInfo) < 0) { + Py_DECREF(versionInfo); + return -1; + } + } + /* XXX When Expat supports some way of figuring out how it was + compiled, this should check and set native_encoding + appropriately. + */ + if (PyModule_AddStringConstant(mod, "native_encoding", "UTF-8") < 0) { + return -1; + } + + if (add_errors_module(mod) < 0) { + return -1; + } + + if (add_model_module(mod) < 0) { + return -1; + } + +#if XML_COMBINED_VERSION > 19505 + { + const XML_Feature *features = XML_GetFeatureList(); + PyObject *list = PyList_New(0); + if (list == NULL) { + return -1; + } + + int i = 0; + for (; features[i].feature != XML_FEATURE_END; ++i) { + PyObject *item = Py_BuildValue("si", features[i].name, + features[i].value); + if (item == NULL) { + Py_DECREF(list); + return -1; + } + int ok = PyList_Append(list, item); + Py_DECREF(item); + if (ok < 0) { + PyErr_Clear(); + Py_DECREF(list); + return -1; + } + } + if (PyModule_AddObject(mod, "features", list) < 0) { + Py_DECREF(list); + return -1; + } + } +#endif + +#define MYCONST(c) do { \ + if (PyModule_AddIntConstant(mod, #c, c) < 0) { \ + return -1; \ + } \ + } while(0) + + MYCONST(XML_PARAM_ENTITY_PARSING_NEVER); + MYCONST(XML_PARAM_ENTITY_PARSING_UNLESS_STANDALONE); + MYCONST(XML_PARAM_ENTITY_PARSING_ALWAYS); +#undef MYCONST static struct PyExpat_CAPI capi; /* initialize pyexpat dispatch table */ @@ -1917,20 +1938,15 @@ pyexpat_exec(PyObject *mod) /* export using capsule */ PyObject *capi_object = PyCapsule_New(&capi, PyExpat_CAPSULE_NAME, NULL); if (capi_object == NULL) { - goto error; + return -1; } if (PyModule_AddObject(mod, "expat_CAPI", capi_object) < 0) { Py_DECREF(capi_object); - goto error; + return -1; } return 0; - -error: - Py_XDECREF(codes_dict); - Py_XDECREF(rev_codes_dict); - return -1; } static PyModuleDef_Slot pyexpat_slots[] = { From f99b31c2ec36d1729d9f20184c1244233201b218 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Sun, 1 Nov 2020 19:59:18 -0600 Subject: [PATCH 6/8] address comments --- Modules/pyexpat.c | 80 +++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 554376d0530cdf..3a12ba1711d5db 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1794,6 +1794,37 @@ add_model_module(PyObject *mod) return 0; } +static int add_features(PyObject *mod) { + #if XML_COMBINED_VERSION > 19505 + PyObject *list = PyList_New(0); + if (list == NULL) { + return -1; + } + + const XML_Feature *features = XML_GetFeatureList(); + for (size_t i = 0; features[i].feature != XML_FEATURE_END; ++i) { + PyObject *item = Py_BuildValue("si", features[i].name, + features[i].value); + if (item == NULL) { + goto error; + } + int ok = PyList_Append(list, item); + Py_DECREF(item); + if (ok < 0) { + goto error; + } + } + if (PyModule_AddObject(mod, "features", list) < 0) { + goto error; + } + +#endif + return 0; +error: + Py_DECREF(list); + return -1; +} + static int pyexpat_exec(PyObject *mod) { @@ -1862,36 +1893,9 @@ pyexpat_exec(PyObject *mod) return -1; } -#if XML_COMBINED_VERSION > 19505 - { - const XML_Feature *features = XML_GetFeatureList(); - PyObject *list = PyList_New(0); - if (list == NULL) { - return -1; - } - - int i = 0; - for (; features[i].feature != XML_FEATURE_END; ++i) { - PyObject *item = Py_BuildValue("si", features[i].name, - features[i].value); - if (item == NULL) { - Py_DECREF(list); - return -1; - } - int ok = PyList_Append(list, item); - Py_DECREF(item); - if (ok < 0) { - PyErr_Clear(); - Py_DECREF(list); - return -1; - } - } - if (PyModule_AddObject(mod, "features", list) < 0) { - Py_DECREF(list); - return -1; - } + if (add_features(mod) < 0) { + return -1; } -#endif #define MYCONST(c) do { \ if (PyModule_AddIntConstant(mod, #c, c) < 0) { \ @@ -1949,24 +1953,26 @@ pyexpat_exec(PyObject *mod) return 0; } -static PyModuleDef_Slot pyexpat_slots[] = { - {Py_mod_exec, pyexpat_exec}, - {0, NULL} -}; - static struct PyModuleDef pyexpatmodule = { PyModuleDef_HEAD_INIT, .m_name = MODULE_NAME, .m_doc = pyexpat_module_documentation, - .m_size = 0, + .m_size = -1, .m_methods = pyexpat_methods, - .m_slots = pyexpat_slots }; PyMODINIT_FUNC PyInit_pyexpat(void) { - return PyModuleDef_Init(&pyexpatmodule); + PyObject *mod = PyModule_Create(&pyexpatmodule); + if (mod == NULL) + return NULL; + + if (pyexpat_exec(mod) < 0) { + Py_DECREF(mod); + return NULL; + } + return mod; } static void From 25c10709f1bf7a3677de50700b2ebc31536cc25e Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Mon, 2 Nov 2020 19:43:00 -0600 Subject: [PATCH 7/8] update based on review --- Modules/pyexpat.c | 166 ++++++++++++++++++++++++++-------------------- 1 file changed, 93 insertions(+), 73 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 3a12ba1711d5db..2f1c72eb006a95 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1640,18 +1640,52 @@ add_submodule(PyObject *mod, const char *fullname) Py_DECREF(mod_name); return NULL; } + Py_DECREF(mod_name); /* gives away the reference to the submodule */ if (PyModule_AddObject(mod, name, submodule) < 0) { Py_DECREF(submodule); - Py_DECREF(mod_name); return NULL; } - Py_DECREF(mod_name); return submodule; } +static int +add_error(PyObject *errors_module, PyObject *codes_dict, + PyObject *rev_codes_dict, const char *name, int value) +{ + const char *error_string = XML_ErrorString(value); + if (PyModule_AddStringConstant(errors_module, name, error_string) < 0) { + return -1; + } + + PyObject *num = PyLong_FromLong(value); + if (num == NULL) { + return -1; + } + + if (PyDict_SetItemString(codes_dict, error_string, num) < 0) { + Py_DECREF(num); + return -1; + } + + PyObject *str = PyUnicode_FromString(error_string); + if (str == NULL) { + Py_DECREF(num); + return -1; + } + + int res = PyDict_SetItem(rev_codes_dict, num, str); + Py_CLEAR(str); + Py_CLEAR(num); + if (res < 0) { + return -1; + } + + return 0; +} + static int add_errors_module(PyObject *mod) { @@ -1666,76 +1700,53 @@ add_errors_module(PyObject *mod) goto error; } - int res; - PyObject *tmpnum, *tmpstr; -#define MYCONST(name) do { \ - if (PyModule_AddStringConstant(errors_module, #name, \ - XML_ErrorString(name)) < 0) {\ - goto error; \ - } \ - tmpnum = PyLong_FromLong(name); \ - if (tmpnum == NULL) { \ - goto error; \ - } \ - res = PyDict_SetItemString(codes_dict, \ - XML_ErrorString(name), tmpnum); \ - if (res < 0) { \ - Py_DECREF(tmpnum); \ - goto error; \ - } \ - tmpstr = PyUnicode_FromString(XML_ErrorString(name)); \ - if (tmpstr == NULL) { \ - Py_DECREF(tmpnum); \ - goto error; \ - } \ - res = PyDict_SetItem(rev_codes_dict, tmpnum, tmpstr); \ - Py_CLEAR(tmpstr); \ - Py_CLEAR(tmpnum); \ - if (res < 0) { \ +#define ADD_CONST(name) do { \ + if (add_error(errors_module, codes_dict, rev_codes_dict, \ + #name, name) < 0) { \ goto error; \ } \ } while(0) - - MYCONST(XML_ERROR_NO_MEMORY); - MYCONST(XML_ERROR_SYNTAX); - MYCONST(XML_ERROR_NO_ELEMENTS); - MYCONST(XML_ERROR_INVALID_TOKEN); - MYCONST(XML_ERROR_UNCLOSED_TOKEN); - MYCONST(XML_ERROR_PARTIAL_CHAR); - MYCONST(XML_ERROR_TAG_MISMATCH); - MYCONST(XML_ERROR_DUPLICATE_ATTRIBUTE); - MYCONST(XML_ERROR_JUNK_AFTER_DOC_ELEMENT); - MYCONST(XML_ERROR_PARAM_ENTITY_REF); - MYCONST(XML_ERROR_UNDEFINED_ENTITY); - MYCONST(XML_ERROR_RECURSIVE_ENTITY_REF); - MYCONST(XML_ERROR_ASYNC_ENTITY); - MYCONST(XML_ERROR_BAD_CHAR_REF); - MYCONST(XML_ERROR_BINARY_ENTITY_REF); - MYCONST(XML_ERROR_ATTRIBUTE_EXTERNAL_ENTITY_REF); - MYCONST(XML_ERROR_MISPLACED_XML_PI); - MYCONST(XML_ERROR_UNKNOWN_ENCODING); - MYCONST(XML_ERROR_INCORRECT_ENCODING); - MYCONST(XML_ERROR_UNCLOSED_CDATA_SECTION); - MYCONST(XML_ERROR_EXTERNAL_ENTITY_HANDLING); - MYCONST(XML_ERROR_NOT_STANDALONE); - MYCONST(XML_ERROR_UNEXPECTED_STATE); - MYCONST(XML_ERROR_ENTITY_DECLARED_IN_PE); - MYCONST(XML_ERROR_FEATURE_REQUIRES_XML_DTD); - MYCONST(XML_ERROR_CANT_CHANGE_FEATURE_ONCE_PARSING); + ADD_CONST(XML_ERROR_NO_MEMORY); + ADD_CONST(XML_ERROR_SYNTAX); + ADD_CONST(XML_ERROR_NO_ELEMENTS); + ADD_CONST(XML_ERROR_INVALID_TOKEN); + ADD_CONST(XML_ERROR_UNCLOSED_TOKEN); + ADD_CONST(XML_ERROR_PARTIAL_CHAR); + ADD_CONST(XML_ERROR_TAG_MISMATCH); + ADD_CONST(XML_ERROR_DUPLICATE_ATTRIBUTE); + ADD_CONST(XML_ERROR_JUNK_AFTER_DOC_ELEMENT); + ADD_CONST(XML_ERROR_PARAM_ENTITY_REF); + ADD_CONST(XML_ERROR_UNDEFINED_ENTITY); + ADD_CONST(XML_ERROR_RECURSIVE_ENTITY_REF); + ADD_CONST(XML_ERROR_ASYNC_ENTITY); + ADD_CONST(XML_ERROR_BAD_CHAR_REF); + ADD_CONST(XML_ERROR_BINARY_ENTITY_REF); + ADD_CONST(XML_ERROR_ATTRIBUTE_EXTERNAL_ENTITY_REF); + ADD_CONST(XML_ERROR_MISPLACED_XML_PI); + ADD_CONST(XML_ERROR_UNKNOWN_ENCODING); + ADD_CONST(XML_ERROR_INCORRECT_ENCODING); + ADD_CONST(XML_ERROR_UNCLOSED_CDATA_SECTION); + ADD_CONST(XML_ERROR_EXTERNAL_ENTITY_HANDLING); + ADD_CONST(XML_ERROR_NOT_STANDALONE); + ADD_CONST(XML_ERROR_UNEXPECTED_STATE); + ADD_CONST(XML_ERROR_ENTITY_DECLARED_IN_PE); + ADD_CONST(XML_ERROR_FEATURE_REQUIRES_XML_DTD); + ADD_CONST(XML_ERROR_CANT_CHANGE_FEATURE_ONCE_PARSING); /* Added in Expat 1.95.7. */ - MYCONST(XML_ERROR_UNBOUND_PREFIX); + ADD_CONST(XML_ERROR_UNBOUND_PREFIX); /* Added in Expat 1.95.8. */ - MYCONST(XML_ERROR_UNDECLARING_PREFIX); - MYCONST(XML_ERROR_INCOMPLETE_PE); - MYCONST(XML_ERROR_XML_DECL); - MYCONST(XML_ERROR_TEXT_DECL); - MYCONST(XML_ERROR_PUBLICID); - MYCONST(XML_ERROR_SUSPENDED); - MYCONST(XML_ERROR_NOT_SUSPENDED); - MYCONST(XML_ERROR_ABORTED); - MYCONST(XML_ERROR_FINISHED); - MYCONST(XML_ERROR_SUSPEND_PE); + ADD_CONST(XML_ERROR_UNDECLARING_PREFIX); + ADD_CONST(XML_ERROR_INCOMPLETE_PE); + ADD_CONST(XML_ERROR_XML_DECL); + ADD_CONST(XML_ERROR_TEXT_DECL); + ADD_CONST(XML_ERROR_PUBLICID); + ADD_CONST(XML_ERROR_SUSPENDED); + ADD_CONST(XML_ERROR_NOT_SUSPENDED); + ADD_CONST(XML_ERROR_ABORTED); + ADD_CONST(XML_ERROR_FINISHED); + ADD_CONST(XML_ERROR_SUSPEND_PE); +#undef ADD_CONST if (PyModule_AddStringConstant(errors_module, "__doc__", "Constants used to describe " @@ -1743,17 +1754,23 @@ add_errors_module(PyObject *mod) goto error; } + Py_INCREF(codes_dict); if (PyModule_AddObject(errors_module, "codes", codes_dict) < 0) { + Py_DECREF(codes_dict); goto error; } + Py_CLEAR(codes_dict); + + Py_INCREF(rev_codes_dict); if (PyModule_AddObject(errors_module, "messages", rev_codes_dict) < 0) { + Py_DECREF(rev_codes_dict); goto error; } - #undef MYCONST + Py_CLEAR(rev_codes_dict); return 0; - error: +error: Py_XDECREF(codes_dict); Py_XDECREF(rev_codes_dict); return -1; @@ -1794,8 +1811,9 @@ add_model_module(PyObject *mod) return 0; } -static int add_features(PyObject *mod) { - #if XML_COMBINED_VERSION > 19505 +#if XML_COMBINED_VERSION > 19505 +static int +add_features(PyObject *mod) { PyObject *list = PyList_New(0); if (list == NULL) { return -1; @@ -1804,7 +1822,7 @@ static int add_features(PyObject *mod) { const XML_Feature *features = XML_GetFeatureList(); for (size_t i = 0; features[i].feature != XML_FEATURE_END; ++i) { PyObject *item = Py_BuildValue("si", features[i].name, - features[i].value); + features[i].value); if (item == NULL) { goto error; } @@ -1817,13 +1835,13 @@ static int add_features(PyObject *mod) { if (PyModule_AddObject(mod, "features", list) < 0) { goto error; } - -#endif + return 0; error: Py_DECREF(list); return -1; } +#endif static int pyexpat_exec(PyObject *mod) @@ -1893,9 +1911,11 @@ pyexpat_exec(PyObject *mod) return -1; } +#if XML_COMBINED_VERSION > 19505 if (add_features(mod) < 0) { return -1; } +#endif #define MYCONST(c) do { \ if (PyModule_AddIntConstant(mod, #c, c) < 0) { \ From e9055969b019c1828e82d893f9c8a0d00abaf829 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Tue, 3 Nov 2020 20:34:41 -0600 Subject: [PATCH 8/8] readability: --- Modules/pyexpat.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 2f1c72eb006a95..e401079fe20add 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1677,8 +1677,8 @@ add_error(PyObject *errors_module, PyObject *codes_dict, } int res = PyDict_SetItem(rev_codes_dict, num, str); - Py_CLEAR(str); - Py_CLEAR(num); + Py_DECREF(str); + Py_DECREF(num); if (res < 0) { return -1; } @@ -1813,7 +1813,8 @@ add_model_module(PyObject *mod) #if XML_COMBINED_VERSION > 19505 static int -add_features(PyObject *mod) { +add_features(PyObject *mod) +{ PyObject *list = PyList_New(0); if (list == NULL) { return -1; @@ -1835,8 +1836,8 @@ add_features(PyObject *mod) { if (PyModule_AddObject(mod, "features", list) < 0) { goto error; } - return 0; + error: Py_DECREF(list); return -1;