From 014c384ad2e4b82bf430f53b8f7b516b51036b29 Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Mon, 24 Sep 2018 09:27:46 +0200 Subject: [PATCH 01/12] Fix PyStructSequence_NewType --- Objects/structseq.c | 195 ++++++++++++++++++++++++-------------------- 1 file changed, 108 insertions(+), 87 deletions(-) diff --git a/Objects/structseq.c b/Objects/structseq.c index 1705837f71fa57..4edcf747895353 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -287,78 +287,48 @@ static PyMethodDef structseq_methods[] = { {NULL, NULL} }; -static PyTypeObject _struct_sequence_template = { - PyVarObject_HEAD_INIT(&PyType_Type, 0) - NULL, /* tp_name */ - sizeof(PyStructSequence) - sizeof(PyObject *), /* tp_basicsize */ - sizeof(PyObject *), /* tp_itemsize */ - (destructor)structseq_dealloc, /* tp_dealloc */ - 0, /* tp_print */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_reserved */ - (reprfunc)structseq_repr, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - 0, /* tp_call */ - 0, /* tp_str */ - 0, /* tp_getattro */ - 0, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT, /* tp_flags */ - NULL, /* tp_doc */ - 0, /* tp_traverse */ - 0, /* tp_clear */ - 0, /* tp_richcompare */ - 0, /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - structseq_methods, /* tp_methods */ - NULL, /* tp_members */ - 0, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - 0, /* tp_descr_get */ - 0, /* tp_descr_set */ - 0, /* tp_dictoffset */ - 0, /* tp_init */ - 0, /* tp_alloc */ - structseq_new, /* tp_new */ -}; +static void count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_members, Py_ssize_t *n_unnamed_members) { + Py_ssize_t i; -int -PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) -{ - PyObject *dict; - PyMemberDef* members; - Py_ssize_t n_members, n_unnamed_members, i, k; + *n_unnamed_members = 0; + for (i = 0; desc->fields[i].name != NULL; ++i) + if (desc->fields[i].name == PyStructSequence_UnnamedField) + (*n_unnamed_members)++; + (*n_members) = i; +} + +static int initialize_structseq_dict(PyStructSequence_Desc *desc, PyObject* dict) { PyObject *v; + Py_ssize_t n_members, n_unnamed_members; -#ifdef Py_TRACE_REFS - /* if the type object was chained, unchain it first - before overwriting its storage */ - if (type->ob_base.ob_base._ob_next) { - _Py_ForgetReference((PyObject*)type); - } -#endif +#define SET_DICT_FROM_SIZE(key, value) \ + do { \ + v = PyLong_FromSsize_t(value); \ + if (v == NULL) \ + return -1; \ + if (PyDict_SetItemString(dict, key, v) < 0) { \ + Py_DECREF(v); \ + return -1; \ + } \ + Py_DECREF(v); \ + } while (0) - n_unnamed_members = 0; - for (i = 0; desc->fields[i].name != NULL; ++i) - if (desc->fields[i].name == PyStructSequence_UnnamedField) - n_unnamed_members++; - n_members = i; + count_members(desc, &n_members, &n_unnamed_members); + SET_DICT_FROM_SIZE(visible_length_key, desc->n_in_sequence); + SET_DICT_FROM_SIZE(real_length_key, n_members); + SET_DICT_FROM_SIZE(unnamed_fields_key, n_unnamed_members); + return 0; +} - memcpy(type, &_struct_sequence_template, sizeof(PyTypeObject)); - type->tp_base = &PyTuple_Type; - type->tp_name = desc->name; - type->tp_doc = desc->doc; +static PyMemberDef* initialize_members(PyStructSequence_Desc *desc) { + PyMemberDef* members; + Py_ssize_t n_members, n_unnamed_members, i, k; - members = PyMem_NEW(PyMemberDef, n_members-n_unnamed_members+1); + count_members(desc, &n_members, &n_unnamed_members); + members = PyMem_NEW(PyMemberDef, n_members-n_unnamed_members + 1); if (members == NULL) { PyErr_NoMemory(); - return -1; + return NULL; } for (i = k = 0; i < n_members; ++i) { @@ -373,29 +343,45 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) k++; } members[k].name = NULL; + return members; +} +int +PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) +{ + PyMemberDef* members; + +#ifdef Py_TRACE_REFS + /* if the type object was chained, unchain it first + before overwriting its storage */ + if (type->ob_base.ob_base._ob_next) { + _Py_ForgetReference((PyObject*)type); + } +#endif + + Py_REFCNT(type) = 1; + type->tp_name = desc->name; + type->tp_basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); + type->tp_itemsize = sizeof(PyObject *); + type->tp_dealloc = (destructor)structseq_dealloc; + type->tp_repr = (reprfunc)structseq_repr; + type->tp_doc = desc->doc; + type->tp_base = &PyTuple_Type; + type->tp_methods = structseq_methods; + type->tp_new = structseq_new; + type->tp_flags = Py_TPFLAGS_DEFAULT; + + members = initialize_members(desc); + if (members == NULL) + return -1; type->tp_members = members; if (PyType_Ready(type) < 0) return -1; Py_INCREF(type); - dict = type->tp_dict; -#define SET_DICT_FROM_SIZE(key, value) \ - do { \ - v = PyLong_FromSsize_t(value); \ - if (v == NULL) \ - return -1; \ - if (PyDict_SetItemString(dict, key, v) < 0) { \ - Py_DECREF(v); \ - return -1; \ - } \ - Py_DECREF(v); \ - } while (0) - - SET_DICT_FROM_SIZE(visible_length_key, desc->n_in_sequence); - SET_DICT_FROM_SIZE(real_length_key, n_members); - SET_DICT_FROM_SIZE(unnamed_fields_key, n_unnamed_members); + if (initialize_structseq_dict(desc, type->tp_dict) < 0) + return -1; return 0; } @@ -409,16 +395,51 @@ PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc) PyTypeObject* PyStructSequence_NewType(PyStructSequence_Desc *desc) { - PyTypeObject *result; + PyObject* type; + PyObject* bases; + PyMemberDef* members; + + PyType_Spec* spec = PyMem_NEW(PyType_Spec, 1); + spec->name = desc->name; + spec->basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); + spec->itemsize = sizeof(PyObject *); + spec->flags = Py_TPFLAGS_DEFAULT; + spec->slots = PyMem_NEW(PyType_Slot, 6); + + spec->slots[0].slot = Py_tp_dealloc; + spec->slots[0].pfunc = (destructor)structseq_dealloc; + + spec->slots[1].slot = Py_tp_repr; + spec->slots[1].pfunc = (reprfunc)structseq_repr; - result = (PyTypeObject*)PyType_GenericAlloc(&PyType_Type, 0); - if (result == NULL) + spec->slots[2].slot = Py_tp_doc; + spec->slots[2].pfunc = (void*)desc->doc; + + spec->slots[3].slot = Py_tp_methods; + spec->slots[3].pfunc = structseq_methods; + + spec->slots[4].slot = Py_tp_new; + spec->slots[4].pfunc = structseq_new; + + members = initialize_members(desc); + if (members == NULL) return NULL; - if (PyStructSequence_InitType2(result, desc) < 0) { - Py_DECREF(result); + spec->slots[5].slot = Py_tp_members; + spec->slots[5].pfunc = members; + + spec->slots[6].slot = 0; + spec->slots[6].pfunc = 0; + + bases = PyTuple_Pack(1, &PyTuple_Type); + type = PyType_FromSpecWithBases(spec, bases); + if (type == NULL) return NULL; - } - return result; + Py_INCREF(type); + + if (initialize_structseq_dict(desc, ((PyTypeObject *)type)->tp_dict) < 0) + return NULL; + + return type; } int _PyStructSequence_Init(void) From 8209ac3a8fe578bf82c75980f86773b559112443 Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Mon, 1 Oct 2018 16:17:45 -0700 Subject: [PATCH 02/12] posixmodule using NewType --- Modules/posixmodule.c | 85 +++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 40 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 0307436e3d599d..8170c3a6783e05 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1948,14 +1948,14 @@ static PyStructSequence_Desc waitid_result_desc = { waitid_result_fields, 5 }; -static PyTypeObject WaitidResultType; +static PyTypeObject* WaitidResultType; #endif static int initialized; -static PyTypeObject StatResultType; -static PyTypeObject StatVFSResultType; +static PyTypeObject* StatResultType; +static PyTypeObject* StatVFSResultType; #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM) -static PyTypeObject SchedParamType; +static PyTypeObject* SchedParamType; #endif static newfunc structseq_new; @@ -2029,7 +2029,7 @@ static PyObject* _pystat_fromstructstat(STRUCT_STAT *st) { unsigned long ansec, mnsec, cnsec; - PyObject *v = PyStructSequence_New(&StatResultType); + PyObject *v = PyStructSequence_New(StatResultType); if (v == NULL) return NULL; @@ -4407,7 +4407,7 @@ static PyStructSequence_Desc uname_result_desc = { 5 }; -static PyTypeObject UnameResultType; +static PyTypeObject* UnameResultType; #ifdef HAVE_UNAME @@ -4435,7 +4435,7 @@ os_uname_impl(PyObject *module) if (res < 0) return posix_error(); - value = PyStructSequence_New(&UnameResultType); + value = PyStructSequence_New(UnameResultType); if (value == NULL) return NULL; @@ -5941,7 +5941,7 @@ os_sched_getscheduler_impl(PyObject *module, pid_t pid) #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM) /*[clinic input] -class os.sched_param "PyObject *" "&SchedParamType" +class os.sched_param "PyObject *" "SchedParamType" @classmethod os.sched_param.__new__ @@ -5986,7 +5986,7 @@ convert_sched_param(PyObject *param, struct sched_param *res) { long priority; - if (Py_TYPE(param) != &SchedParamType) { + if (Py_TYPE(param) != SchedParamType) { PyErr_SetString(PyExc_TypeError, "must have a sched_param object"); return 0; } @@ -6057,7 +6057,7 @@ os_sched_getparam_impl(PyObject *module, pid_t pid) if (sched_getparam(pid, ¶m)) return posix_error(); - result = PyStructSequence_New(&SchedParamType); + result = PyStructSequence_New(SchedParamType); if (!result) return NULL; priority = PyLong_FromLong(param.sched_priority); @@ -7422,7 +7422,7 @@ os_waitid_impl(PyObject *module, idtype_t idtype, id_t id, int options) if (si.si_pid == 0) Py_RETURN_NONE; - result = PyStructSequence_New(&WaitidResultType); + result = PyStructSequence_New(WaitidResultType); if (!result) return NULL; @@ -7857,7 +7857,7 @@ static PyStructSequence_Desc times_result_desc = { 5 }; -static PyTypeObject TimesResultType; +static PyTypeObject* TimesResultType; #ifdef MS_WINDOWS #define HAVE_TIMES /* mandatory, for the method table */ @@ -7870,7 +7870,7 @@ build_times_result(double user, double system, double children_user, double children_system, double elapsed) { - PyObject *value = PyStructSequence_New(&TimesResultType); + PyObject *value = PyStructSequence_New(TimesResultType); if (value == NULL) return NULL; @@ -9950,7 +9950,7 @@ os_WSTOPSIG_impl(PyObject *module, int status) static PyObject* _pystatvfs_fromstructstatvfs(struct statvfs st) { - PyObject *v = PyStructSequence_New(&StatVFSResultType); + PyObject *v = PyStructSequence_New(StatVFSResultType); if (v == NULL) return NULL; @@ -11703,7 +11703,7 @@ os_urandom_impl(PyObject *module, Py_ssize_t size) /* Terminal size querying */ -static PyTypeObject TerminalSizeType; +static PyTypeObject* TerminalSizeType; PyDoc_STRVAR(TerminalSize_docstring, "A tuple of (columns, lines) for holding terminal window size"); @@ -11795,7 +11795,7 @@ get_terminal_size(PyObject *self, PyObject *args) } #endif /* TERMSIZE_USE_CONIO */ - termsize = PyStructSequence_New(&TerminalSizeType); + termsize = PyStructSequence_New(TerminalSizeType); if (termsize == NULL) return NULL; PyStructSequence_SET_ITEM(termsize, 0, PyLong_FromLong(columns)); @@ -13912,7 +13912,8 @@ INITFUNC(void) if (!initialized) { #if defined(HAVE_WAITID) && !defined(__APPLE__) waitid_result_desc.name = MODNAME ".waitid_result"; - if (PyStructSequence_InitType2(&WaitidResultType, &waitid_result_desc) < 0) + WaitidResultType = PyStructSequence_NewType(&waitid_result_desc); + if (WaitidResultType == NULL) return NULL; #endif @@ -13920,14 +13921,15 @@ INITFUNC(void) stat_result_desc.fields[7].name = PyStructSequence_UnnamedField; stat_result_desc.fields[8].name = PyStructSequence_UnnamedField; stat_result_desc.fields[9].name = PyStructSequence_UnnamedField; - if (PyStructSequence_InitType2(&StatResultType, &stat_result_desc) < 0) + StatResultType = PyStructSequence_NewType(&stat_result_desc); + if (StatResultType == NULL) return NULL; - structseq_new = StatResultType.tp_new; - StatResultType.tp_new = statresult_new; + structseq_new = StatResultType->tp_new; + StatResultType->tp_new = statresult_new; statvfs_result_desc.name = "os.statvfs_result"; /* see issue #19209 */ - if (PyStructSequence_InitType2(&StatVFSResultType, - &statvfs_result_desc) < 0) + StatVFSResultType = PyStructSequence_NewType(&statvfs_result_desc); + if (StatVFSResultType == NULL) return NULL; #ifdef NEED_TICKS_PER_SECOND # if defined(HAVE_SYSCONF) && defined(_SC_CLK_TCK) @@ -13941,14 +13943,15 @@ INITFUNC(void) #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM) sched_param_desc.name = MODNAME ".sched_param"; - if (PyStructSequence_InitType2(&SchedParamType, &sched_param_desc) < 0) + SchedParamType = PyStructSequence_NewType(&sched_param_desc); + if (SchedParamType == NULL) return NULL; - SchedParamType.tp_new = os_sched_param; + SchedParamType->tp_new = os_sched_param; #endif /* initialize TerminalSize_info */ - if (PyStructSequence_InitType2(&TerminalSizeType, - &TerminalSize_desc) < 0) + TerminalSizeType = PyStructSequence_NewType(&TerminalSize_desc); + if (TerminalSizeType == NULL) return NULL; /* initialize scandir types */ @@ -13958,29 +13961,31 @@ INITFUNC(void) return NULL; } #if defined(HAVE_WAITID) && !defined(__APPLE__) - Py_INCREF((PyObject*) &WaitidResultType); - PyModule_AddObject(m, "waitid_result", (PyObject*) &WaitidResultType); + Py_INCREF((PyObject*) WaitidResultType); + PyModule_AddObject(m, "waitid_result", (PyObject*) WaitidResultType); #endif - Py_INCREF((PyObject*) &StatResultType); - PyModule_AddObject(m, "stat_result", (PyObject*) &StatResultType); - Py_INCREF((PyObject*) &StatVFSResultType); + Py_INCREF((PyObject*) StatResultType); + PyModule_AddObject(m, "stat_result", (PyObject*) StatResultType); + Py_INCREF((PyObject*) StatVFSResultType); PyModule_AddObject(m, "statvfs_result", - (PyObject*) &StatVFSResultType); + (PyObject*) StatVFSResultType); #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) - Py_INCREF(&SchedParamType); - PyModule_AddObject(m, "sched_param", (PyObject *)&SchedParamType); + Py_INCREF(SchedParamType); + PyModule_AddObject(m, "sched_param", (PyObject *)SchedParamType); #endif times_result_desc.name = MODNAME ".times_result"; - if (PyStructSequence_InitType2(&TimesResultType, ×_result_desc) < 0) + TimesResultType = PyStructSequence_NewType(×_result_desc); + if (TimesResultType == NULL) return NULL; - PyModule_AddObject(m, "times_result", (PyObject *)&TimesResultType); + PyModule_AddObject(m, "times_result", (PyObject *)TimesResultType); uname_result_desc.name = MODNAME ".uname_result"; - if (PyStructSequence_InitType2(&UnameResultType, &uname_result_desc) < 0) + UnameResultType = PyStructSequence_NewType(&uname_result_desc); + if (UnameResultType == NULL) return NULL; - PyModule_AddObject(m, "uname_result", (PyObject *)&UnameResultType); + PyModule_AddObject(m, "uname_result", (PyObject *)UnameResultType); #ifdef __APPLE__ /* @@ -14020,8 +14025,8 @@ INITFUNC(void) #endif /* __APPLE__ */ - Py_INCREF(&TerminalSizeType); - PyModule_AddObject(m, "terminal_size", (PyObject*) &TerminalSizeType); + Py_INCREF(TerminalSizeType); + PyModule_AddObject(m, "terminal_size", (PyObject*)TerminalSizeType); billion = PyLong_FromLong(1000000000); if (!billion) From 4957f25ba8ba2e63081c03c7656b2d64cfabf822 Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Mon, 1 Oct 2018 19:41:55 -0700 Subject: [PATCH 03/12] Added clinic --- Modules/posixmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 8170c3a6783e05..69a5267333bdab 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -5954,7 +5954,7 @@ Current has only one field: sched_priority"); static PyObject * os_sched_param_impl(PyTypeObject *type, PyObject *sched_priority) -/*[clinic end generated code: output=48f4067d60f48c13 input=73a4c22f7071fc62]*/ +/*[clinic end generated code: output=48f4067d60f48c13 input=ab4de35a9a7811f2]*/ { PyObject *res; From 69b6700463870e6171a598825faf9099dca12da7 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Tue, 2 Oct 2018 08:54:36 -0700 Subject: [PATCH 04/12] Fix a couple of bugs --- Objects/structseq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/structseq.c b/Objects/structseq.c index 4edcf747895353..f238865d36c802 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -404,7 +404,7 @@ PyStructSequence_NewType(PyStructSequence_Desc *desc) spec->basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); spec->itemsize = sizeof(PyObject *); spec->flags = Py_TPFLAGS_DEFAULT; - spec->slots = PyMem_NEW(PyType_Slot, 6); + spec->slots = PyMem_NEW(PyType_Slot, 7); spec->slots[0].slot = Py_tp_dealloc; spec->slots[0].pfunc = (destructor)structseq_dealloc; @@ -439,7 +439,7 @@ PyStructSequence_NewType(PyStructSequence_Desc *desc) if (initialize_structseq_dict(desc, ((PyTypeObject *)type)->tp_dict) < 0) return NULL; - return type; + return (PyTypeObject*)type; } int _PyStructSequence_Init(void) From 9d08386aff347b6bce4efcb3a0d9dd289a2e00b8 Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Tue, 2 Oct 2018 09:11:01 -0700 Subject: [PATCH 05/12] Added news --- .../Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst b/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst new file mode 100644 index 00000000000000..296fc149339bcb --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst @@ -0,0 +1,2 @@ +Fix the implementation of PyStructSequence_NewType in order to create heap +allocated StructSequences. From 5d23b8a91583f308559cafd57590cb661fd0ebd1 Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Thu, 1 Nov 2018 00:13:25 -0700 Subject: [PATCH 06/12] Fix leaks and cleanup code --- Objects/structseq.c | 144 ++++++++++++++++++++++--------------------- Objects/typeobject.c | 24 ++++++-- 2 files changed, 93 insertions(+), 75 deletions(-) diff --git a/Objects/structseq.c b/Objects/structseq.c index f238865d36c802..66d84ec1b3853f 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -287,49 +287,44 @@ static PyMethodDef structseq_methods[] = { {NULL, NULL} }; -static void count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_members, Py_ssize_t *n_unnamed_members) { +static Py_ssize_t +count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_unnamed_members) { Py_ssize_t i; *n_unnamed_members = 0; for (i = 0; desc->fields[i].name != NULL; ++i) if (desc->fields[i].name == PyStructSequence_UnnamedField) (*n_unnamed_members)++; - (*n_members) = i; + return i; } -static int initialize_structseq_dict(PyStructSequence_Desc *desc, PyObject* dict) { +static int +initialize_structseq_dict(PyStructSequence_Desc *desc, PyObject* dict, + Py_ssize_t n_members, Py_ssize_t n_unnamed_members) { PyObject *v; - Py_ssize_t n_members, n_unnamed_members; -#define SET_DICT_FROM_SIZE(key, value) \ - do { \ - v = PyLong_FromSsize_t(value); \ - if (v == NULL) \ - return -1; \ - if (PyDict_SetItemString(dict, key, v) < 0) { \ - Py_DECREF(v); \ - return -1; \ - } \ - Py_DECREF(v); \ +#define SET_DICT_FROM_SIZE(key, value) \ + do { \ + v = PyLong_FromSsize_t(value); \ + if (v == NULL) \ + return -1; \ + if (PyDict_SetItemString(dict, key, v) < 0) { \ + Py_DECREF(v); \ + return -1; \ + } \ + Py_DECREF(v); \ } while (0) - count_members(desc, &n_members, &n_unnamed_members); SET_DICT_FROM_SIZE(visible_length_key, desc->n_in_sequence); SET_DICT_FROM_SIZE(real_length_key, n_members); SET_DICT_FROM_SIZE(unnamed_fields_key, n_unnamed_members); return 0; } -static PyMemberDef* initialize_members(PyStructSequence_Desc *desc) { - PyMemberDef* members; - Py_ssize_t n_members, n_unnamed_members, i, k; - - count_members(desc, &n_members, &n_unnamed_members); - members = PyMem_NEW(PyMemberDef, n_members-n_unnamed_members + 1); - if (members == NULL) { - PyErr_NoMemory(); - return NULL; - } +static void +initialize_members(PyStructSequence_Desc *desc, PyMemberDef* members, + Py_ssize_t n_members) { + Py_ssize_t i, k; for (i = k = 0; i < n_members; ++i) { if (desc->fields[i].name == PyStructSequence_UnnamedField) @@ -343,19 +338,19 @@ static PyMemberDef* initialize_members(PyStructSequence_Desc *desc) { k++; } members[k].name = NULL; - return members; } int PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) { - PyMemberDef* members; + PyMemberDef *members; + Py_ssize_t n_members, n_unnamed_members; #ifdef Py_TRACE_REFS /* if the type object was chained, unchain it first before overwriting its storage */ if (type->ob_base.ob_base._ob_next) { - _Py_ForgetReference((PyObject*)type); + _Py_ForgetReference((PyObject *)type); } #endif @@ -369,19 +364,27 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) type->tp_base = &PyTuple_Type; type->tp_methods = structseq_methods; type->tp_new = structseq_new; - type->tp_flags = Py_TPFLAGS_DEFAULT; + type->tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC; - members = initialize_members(desc); - if (members == NULL) + n_members = count_members(desc, &n_unnamed_members); + members = PyMem_NEW(PyMemberDef, n_members - n_unnamed_members + 1); + if (members == NULL) { + PyErr_NoMemory(); return -1; + } + initialize_members(desc, members, n_members); type->tp_members = members; - if (PyType_Ready(type) < 0) + if (PyType_Ready(type) < 0) { + PyMem_FREE(members); return -1; - Py_INCREF(type); + } - if (initialize_structseq_dict(desc, type->tp_dict) < 0) + if (initialize_structseq_dict( + desc, type->tp_dict, n_members, n_unnamed_members) < 0) { + PyMem_FREE(members); return -1; + } return 0; } @@ -392,54 +395,53 @@ PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc) (void)PyStructSequence_InitType2(type, desc); } -PyTypeObject* +PyTypeObject * PyStructSequence_NewType(PyStructSequence_Desc *desc) { - PyObject* type; - PyObject* bases; - PyMemberDef* members; - - PyType_Spec* spec = PyMem_NEW(PyType_Spec, 1); - spec->name = desc->name; - spec->basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); - spec->itemsize = sizeof(PyObject *); - spec->flags = Py_TPFLAGS_DEFAULT; - spec->slots = PyMem_NEW(PyType_Slot, 7); - - spec->slots[0].slot = Py_tp_dealloc; - spec->slots[0].pfunc = (destructor)structseq_dealloc; - - spec->slots[1].slot = Py_tp_repr; - spec->slots[1].pfunc = (reprfunc)structseq_repr; - - spec->slots[2].slot = Py_tp_doc; - spec->slots[2].pfunc = (void*)desc->doc; - - spec->slots[3].slot = Py_tp_methods; - spec->slots[3].pfunc = structseq_methods; - - spec->slots[4].slot = Py_tp_new; - spec->slots[4].pfunc = structseq_new; + PyMemberDef *members; + PyObject *bases; + PyTypeObject *type; + PyType_Slot slots[7]; + PyType_Spec spec; + Py_ssize_t n_members, n_unnamed_members; - members = initialize_members(desc); - if (members == NULL) + /* Initialize MemberDefs */ + n_members = count_members(desc, &n_unnamed_members); + members = PyMem_NEW(PyMemberDef, n_members - n_unnamed_members + 1); + if (members == NULL) { + PyErr_NoMemory(); return NULL; - spec->slots[5].slot = Py_tp_members; - spec->slots[5].pfunc = members; - - spec->slots[6].slot = 0; - spec->slots[6].pfunc = 0; + } + initialize_members(desc, members, n_members); + + /* Initialize Slots */ + slots[0] = (PyType_Slot){Py_tp_dealloc, (destructor)structseq_dealloc}; + slots[1] = (PyType_Slot){Py_tp_repr, (reprfunc)structseq_repr}; + slots[2] = (PyType_Slot){Py_tp_doc, (void *)desc->doc}; + slots[3] = (PyType_Slot){Py_tp_methods, structseq_methods}; + slots[4] = (PyType_Slot){Py_tp_new, structseq_new}; + slots[5] = (PyType_Slot){Py_tp_members, members}; + slots[6] = (PyType_Slot){0, 0}; + + /* Initialize Spec */ + spec.name = desc->name; + spec.basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); + spec.itemsize = sizeof(PyObject *); + spec.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC; + spec.slots = slots; bases = PyTuple_Pack(1, &PyTuple_Type); - type = PyType_FromSpecWithBases(spec, bases); + type = (PyTypeObject *)PyType_FromSpecWithBases(&spec, bases); + Py_DECREF(bases); + PyMem_FREE(members); if (type == NULL) return NULL; - Py_INCREF(type); - if (initialize_structseq_dict(desc, ((PyTypeObject *)type)->tp_dict) < 0) + if (initialize_structseq_dict( + desc, type->tp_dict, n_members, n_unnamed_members) < 0) return NULL; - return (PyTypeObject*)type; + return type; } int _PyStructSequence_Init(void) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index dedc4f7c876431..7499f6d483fdd4 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2846,15 +2846,24 @@ static const short slotoffsets[] = { PyObject * PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases) { - PyHeapTypeObject *res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, 0); - PyTypeObject *type, *base; + PyHeapTypeObject *res; + PyMemberDef *memb; PyObject *modname; - char *s; - char *res_start = (char*)res; + PyTypeObject *type, *base; + PyType_Slot *slot; + Py_ssize_t nslots; + char *s, *res_start; + nslots = 0; + for (slot = spec->slots; slot->slot; slot++) + if (slot->slot == Py_tp_members) + for (memb = slot->pfunc; memb->name != NULL; memb++) + nslots++; + res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, nslots); if (res == NULL) return NULL; + res_start = (char*)res; if (spec->name == NULL) { PyErr_SetString(PyExc_SystemError, @@ -2950,6 +2959,13 @@ PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases) memcpy(tp_doc, old_doc, len); type->tp_doc = tp_doc; } + + /* Move the slots to the heap type itself */ + if (slot->slot == Py_tp_members) { + size_t len = Py_TYPE(type)->tp_itemsize * nslots; + memcpy(PyHeapType_GET_MEMBERS(res), slot->pfunc, len); + type->tp_members = PyHeapType_GET_MEMBERS(res); + } } if (type->tp_dealloc == NULL) { /* It's a heap type, so needs the heap types' dealloc. From 1e2b3dac5a7725b1c64f216092620de404520132 Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Thu, 1 Nov 2018 02:10:18 -0700 Subject: [PATCH 07/12] Added refcnt test --- Lib/test/test_capi.py | 22 ++++++++++++++++++++++ Modules/_testcapimodule.c | 26 ++++++++++++++++++++++++++ Objects/structseq.c | 4 +++- 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index b3600ebe993de5..15e5314584f20f 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -2,6 +2,7 @@ # these are all functions _testcapi exports whose name begins with 'test_'. from collections import OrderedDict +import gc import os import pickle import random @@ -333,6 +334,27 @@ def test_negative_refcount(self): br'_Py_NegativeRefcount: Assertion ".*" failed; ' br'object has negative ref count') + @unittest.skipUnless(hasattr(_testcapi, 'structseq_newtype_doesnt_leak'), + 'need _testcapi.structseq_newtype_doesnt_leak') + def test_structseq_newtype_doesnt_leak(self): + # Warmup the caches + for i in range(10): + _testcapi.structseq_newtype_doesnt_leak() + + # Iterate multiple times + gc.collect() + refcounts_before = sys.gettotalrefcount() + blocks_before = sys.getallocatedblocks() + for i in range(3000): + _testcapi.structseq_newtype_doesnt_leak() + gc.collect() + refcounts_after = sys.gettotalrefcount() + blocks_after = sys.getallocatedblocks() + + # Check results + self.assertAlmostEqual(refcounts_after - refcounts_before, 0, delta=10) + self.assertAlmostEqual(blocks_after - blocks_before, 0, delta=10) + class TestPendingCalls(unittest.TestCase): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index e2deb2603da8f2..9722c8201dd77a 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4835,6 +4835,31 @@ negative_refcount(PyObject *self, PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } + +static PyObject * +structseq_newtype_doesnt_leak(PyObject *Py_UNUSED(self), + PyObject *Py_UNUSED(args)) +{ + PyStructSequence_Desc descr; + PyStructSequence_Field descr_fields[3]; + + descr_fields[0] = (PyStructSequence_Field){"foo", "foo value"}; + descr_fields[1] = (PyStructSequence_Field){NULL, "some hidden value"}; + descr_fields[2] = (PyStructSequence_Field){0, NULL}; + + descr.name = "_testcapi.test_descr"; + descr.doc = "This is used to test for memory leaks in NewType"; + descr.fields = descr_fields; + descr.n_in_sequence = 1; + + PyTypeObject* structseq_type = PyStructSequence_NewType(&descr); + assert(structseq_type != NULL); + assert(PyType_Check(structseq_type)); + assert(PyType_FastSubclass(structseq_type, Py_TPFLAGS_TUPLE_SUBCLASS)); + Py_DECREF(structseq_type); + + Py_RETURN_NONE; +} #endif @@ -5065,6 +5090,7 @@ static PyMethodDef TestMethods[] = { {"get_coreconfig", get_coreconfig, METH_NOARGS}, #ifdef Py_REF_DEBUG {"negative_refcount", negative_refcount, METH_NOARGS}, + {"structseq_newtype_doesnt_leak", structseq_newtype_doesnt_leak, METH_NOARGS}, #endif {NULL, NULL} /* sentinel */ }; diff --git a/Objects/structseq.c b/Objects/structseq.c index 66d84ec1b3853f..d8c43cfdecd860 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -354,7 +354,7 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) } #endif - Py_REFCNT(type) = 1; + Py_REFCNT(type) = 0; type->tp_name = desc->name; type->tp_basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); type->tp_itemsize = sizeof(PyObject *); @@ -379,10 +379,12 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) PyMem_FREE(members); return -1; } + Py_INCREF(type); if (initialize_structseq_dict( desc, type->tp_dict, n_members, n_unnamed_members) < 0) { PyMem_FREE(members); + Py_DECREF(type); return -1; } From 7e614960ea2a5ce7febab0afcddda8e581243cfc Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Fri, 2 Nov 2018 00:08:38 -0700 Subject: [PATCH 08/12] Moved structseq to libregrtest --- Lib/test/test_capi.py | 22 ---------------- Modules/_testcapimodule.c | 53 ++++++++++++++++++++------------------- 2 files changed, 27 insertions(+), 48 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 15e5314584f20f..b3600ebe993de5 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -2,7 +2,6 @@ # these are all functions _testcapi exports whose name begins with 'test_'. from collections import OrderedDict -import gc import os import pickle import random @@ -334,27 +333,6 @@ def test_negative_refcount(self): br'_Py_NegativeRefcount: Assertion ".*" failed; ' br'object has negative ref count') - @unittest.skipUnless(hasattr(_testcapi, 'structseq_newtype_doesnt_leak'), - 'need _testcapi.structseq_newtype_doesnt_leak') - def test_structseq_newtype_doesnt_leak(self): - # Warmup the caches - for i in range(10): - _testcapi.structseq_newtype_doesnt_leak() - - # Iterate multiple times - gc.collect() - refcounts_before = sys.gettotalrefcount() - blocks_before = sys.getallocatedblocks() - for i in range(3000): - _testcapi.structseq_newtype_doesnt_leak() - gc.collect() - refcounts_after = sys.gettotalrefcount() - blocks_after = sys.getallocatedblocks() - - # Check results - self.assertAlmostEqual(refcounts_after - refcounts_before, 0, delta=10) - self.assertAlmostEqual(blocks_after - blocks_before, 0, delta=10) - class TestPendingCalls(unittest.TestCase): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 9722c8201dd77a..cad0f5a842d76a 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3310,6 +3310,31 @@ test_decref_doesnt_leak(PyObject *ob, PyObject *Py_UNUSED(ignored)) Py_RETURN_NONE; } +static PyObject * +test_structseq_newtype_doesnt_leak(PyObject *Py_UNUSED(self), + PyObject *Py_UNUSED(args)) +{ + PyStructSequence_Desc descr; + PyStructSequence_Field descr_fields[3]; + + descr_fields[0] = (PyStructSequence_Field){"foo", "foo value"}; + descr_fields[1] = (PyStructSequence_Field){NULL, "some hidden value"}; + descr_fields[2] = (PyStructSequence_Field){0, NULL}; + + descr.name = "_testcapi.test_descr"; + descr.doc = "This is used to test for memory leaks in NewType"; + descr.fields = descr_fields; + descr.n_in_sequence = 1; + + PyTypeObject* structseq_type = PyStructSequence_NewType(&descr); + assert(structseq_type != NULL); + assert(PyType_Check(structseq_type)); + assert(PyType_FastSubclass(structseq_type, Py_TPFLAGS_TUPLE_SUBCLASS)); + Py_DECREF(structseq_type); + + Py_RETURN_NONE; +} + static PyObject * test_incref_decref_API(PyObject *ob, PyObject *Py_UNUSED(ignored)) { @@ -4835,31 +4860,6 @@ negative_refcount(PyObject *self, PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } - -static PyObject * -structseq_newtype_doesnt_leak(PyObject *Py_UNUSED(self), - PyObject *Py_UNUSED(args)) -{ - PyStructSequence_Desc descr; - PyStructSequence_Field descr_fields[3]; - - descr_fields[0] = (PyStructSequence_Field){"foo", "foo value"}; - descr_fields[1] = (PyStructSequence_Field){NULL, "some hidden value"}; - descr_fields[2] = (PyStructSequence_Field){0, NULL}; - - descr.name = "_testcapi.test_descr"; - descr.doc = "This is used to test for memory leaks in NewType"; - descr.fields = descr_fields; - descr.n_in_sequence = 1; - - PyTypeObject* structseq_type = PyStructSequence_NewType(&descr); - assert(structseq_type != NULL); - assert(PyType_Check(structseq_type)); - assert(PyType_FastSubclass(structseq_type, Py_TPFLAGS_TUPLE_SUBCLASS)); - Py_DECREF(structseq_type); - - Py_RETURN_NONE; -} #endif @@ -4888,6 +4888,8 @@ static PyMethodDef TestMethods[] = { {"test_incref_doesnt_leak", test_incref_doesnt_leak, METH_NOARGS}, {"test_xdecref_doesnt_leak",test_xdecref_doesnt_leak, METH_NOARGS}, {"test_decref_doesnt_leak", test_decref_doesnt_leak, METH_NOARGS}, + {"test_structseq_newtype_doesnt_leak", + test_structseq_newtype_doesnt_leak, METH_NOARGS}, {"test_incref_decref_API", test_incref_decref_API, METH_NOARGS}, {"test_long_and_overflow", test_long_and_overflow, METH_NOARGS}, {"test_long_as_double", test_long_as_double, METH_NOARGS}, @@ -5090,7 +5092,6 @@ static PyMethodDef TestMethods[] = { {"get_coreconfig", get_coreconfig, METH_NOARGS}, #ifdef Py_REF_DEBUG {"negative_refcount", negative_refcount, METH_NOARGS}, - {"structseq_newtype_doesnt_leak", structseq_newtype_doesnt_leak, METH_NOARGS}, #endif {NULL, NULL} /* sentinel */ }; From a19c7e4e541778c991c58c1a92f98874fb5abcc7 Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Fri, 2 Nov 2018 00:13:54 -0700 Subject: [PATCH 09/12] Expanded NEWS to include the original python-dev mail --- .../Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst b/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst index 296fc149339bcb..d78bce76151d1d 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst @@ -1,2 +1,5 @@ Fix the implementation of PyStructSequence_NewType in order to create heap allocated StructSequences. + +Please refer to the following for the reasoning and motivation for this change: +https://mail.python.org/pipermail/python-dev/2018-September/155069.html From 8ac5c68bf912c984f47c51eb6fd232f905bf2172 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Tue, 6 Nov 2018 11:07:47 -0800 Subject: [PATCH 10/12] Addressed encukou comments --- .../2018-10-02-09-10-47.bpo-34784.07hdgD.rst | 3 --- Objects/structseq.c | 15 ++++++++++++-- Objects/typeobject.c | 20 +++++++++++-------- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst b/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst index d78bce76151d1d..296fc149339bcb 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2018-10-02-09-10-47.bpo-34784.07hdgD.rst @@ -1,5 +1,2 @@ Fix the implementation of PyStructSequence_NewType in order to create heap allocated StructSequences. - -Please refer to the following for the reasoning and motivation for this change: -https://mail.python.org/pipermail/python-dev/2018-September/155069.html diff --git a/Objects/structseq.c b/Objects/structseq.c index d8c43cfdecd860..6722bfc4b43ead 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -327,8 +327,12 @@ initialize_members(PyStructSequence_Desc *desc, PyMemberDef* members, Py_ssize_t i, k; for (i = k = 0; i < n_members; ++i) { - if (desc->fields[i].name == PyStructSequence_UnnamedField) + if (desc->fields[i].name == PyStructSequence_UnnamedField) { continue; + } + + /* The names and docstrings in these MemberDefs are statically */ + /* allocated so it is expected that they'll outlive the MemberDef */ members[k].name = desc->fields[i].name; members[k].type = T_OBJECT; members[k].offset = offsetof(PyStructSequence, ob_item) @@ -354,7 +358,12 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) } #endif - Py_REFCNT(type) = 0; + /* PyTypeObject has already been initialized */ + if (Py_REFCNT(type) != 0) { + PyErr_BadInternalCall(); + return -1; + } + type->tp_name = desc->name; type->tp_basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); type->tp_itemsize = sizeof(PyObject *); @@ -426,6 +435,8 @@ PyStructSequence_NewType(PyStructSequence_Desc *desc) slots[6] = (PyType_Slot){0, 0}; /* Initialize Spec */ + /* The name in this PyType_Spec is statically allocated so it is */ + /* expected that it'll outlive the PyType_Spec */ spec.name = desc->name; spec.basicsize = sizeof(PyStructSequence) - sizeof(PyObject *); spec.itemsize = sizeof(PyObject *); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7499f6d483fdd4..0353c99fcd4046 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2852,15 +2852,19 @@ PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases) PyTypeObject *type, *base; PyType_Slot *slot; - Py_ssize_t nslots; + Py_ssize_t nmembers; char *s, *res_start; - nslots = 0; - for (slot = spec->slots; slot->slot; slot++) - if (slot->slot == Py_tp_members) - for (memb = slot->pfunc; memb->name != NULL; memb++) - nslots++; - res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, nslots); + nmembers = 0; + for (slot = spec->slots; slot->slot; slot++) { + if (slot->slot == Py_tp_members) { + for (memb = slot->pfunc; memb->name != NULL; memb++) { + nmembers++; + } + } + } + + res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, nmembers); if (res == NULL) return NULL; res_start = (char*)res; @@ -2962,7 +2966,7 @@ PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases) /* Move the slots to the heap type itself */ if (slot->slot == Py_tp_members) { - size_t len = Py_TYPE(type)->tp_itemsize * nslots; + size_t len = Py_TYPE(type)->tp_itemsize * nmembers; memcpy(PyHeapType_GET_MEMBERS(res), slot->pfunc, len); type->tp_members = PyHeapType_GET_MEMBERS(res); } From 80c03b3bb85b841fd0f1075d671c605323773382 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Tue, 6 Nov 2018 11:18:59 -0800 Subject: [PATCH 11/12] Final cleanups --- Objects/structseq.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Objects/structseq.c b/Objects/structseq.c index 6722bfc4b43ead..922928f9a3b2d9 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -292,9 +292,11 @@ count_members(PyStructSequence_Desc *desc, Py_ssize_t *n_unnamed_members) { Py_ssize_t i; *n_unnamed_members = 0; - for (i = 0; desc->fields[i].name != NULL; ++i) - if (desc->fields[i].name == PyStructSequence_UnnamedField) + for (i = 0; desc->fields[i].name != NULL; ++i) { + if (desc->fields[i].name == PyStructSequence_UnnamedField) { (*n_unnamed_members)++; + } + } return i; } @@ -306,8 +308,9 @@ initialize_structseq_dict(PyStructSequence_Desc *desc, PyObject* dict, #define SET_DICT_FROM_SIZE(key, value) \ do { \ v = PyLong_FromSsize_t(value); \ - if (v == NULL) \ + if (v == NULL) { \ return -1; \ + } \ if (PyDict_SetItemString(dict, key, v) < 0) { \ Py_DECREF(v); \ return -1; \ @@ -447,12 +450,14 @@ PyStructSequence_NewType(PyStructSequence_Desc *desc) type = (PyTypeObject *)PyType_FromSpecWithBases(&spec, bases); Py_DECREF(bases); PyMem_FREE(members); - if (type == NULL) + if (type == NULL) { return NULL; + } if (initialize_structseq_dict( - desc, type->tp_dict, n_members, n_unnamed_members) < 0) + desc, type->tp_dict, n_members, n_unnamed_members) < 0) { return NULL; + } return type; } From 43e92faa90b796421625954489966c0ca09a0ad0 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Tue, 6 Nov 2018 11:31:48 -0800 Subject: [PATCH 12/12] Also update posixmodule if braces --- Modules/posixmodule.c | 21 ++++++++++++++------- Objects/structseq.c | 4 ++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 69a5267333bdab..0efcfe41df0e83 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -13913,8 +13913,9 @@ INITFUNC(void) #if defined(HAVE_WAITID) && !defined(__APPLE__) waitid_result_desc.name = MODNAME ".waitid_result"; WaitidResultType = PyStructSequence_NewType(&waitid_result_desc); - if (WaitidResultType == NULL) + if (WaitidResultType == NULL) { return NULL; + } #endif stat_result_desc.name = "os.stat_result"; /* see issue #19209 */ @@ -13922,15 +13923,17 @@ INITFUNC(void) stat_result_desc.fields[8].name = PyStructSequence_UnnamedField; stat_result_desc.fields[9].name = PyStructSequence_UnnamedField; StatResultType = PyStructSequence_NewType(&stat_result_desc); - if (StatResultType == NULL) + if (StatResultType == NULL) { return NULL; + } structseq_new = StatResultType->tp_new; StatResultType->tp_new = statresult_new; statvfs_result_desc.name = "os.statvfs_result"; /* see issue #19209 */ StatVFSResultType = PyStructSequence_NewType(&statvfs_result_desc); - if (StatVFSResultType == NULL) + if (StatVFSResultType == NULL) { return NULL; + } #ifdef NEED_TICKS_PER_SECOND # if defined(HAVE_SYSCONF) && defined(_SC_CLK_TCK) ticks_per_second = sysconf(_SC_CLK_TCK); @@ -13944,15 +13947,17 @@ INITFUNC(void) #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM) sched_param_desc.name = MODNAME ".sched_param"; SchedParamType = PyStructSequence_NewType(&sched_param_desc); - if (SchedParamType == NULL) + if (SchedParamType == NULL) { return NULL; + } SchedParamType->tp_new = os_sched_param; #endif /* initialize TerminalSize_info */ TerminalSizeType = PyStructSequence_NewType(&TerminalSize_desc); - if (TerminalSizeType == NULL) + if (TerminalSizeType == NULL) { return NULL; + } /* initialize scandir types */ if (PyType_Ready(&ScandirIteratorType) < 0) @@ -13977,14 +13982,16 @@ INITFUNC(void) times_result_desc.name = MODNAME ".times_result"; TimesResultType = PyStructSequence_NewType(×_result_desc); - if (TimesResultType == NULL) + if (TimesResultType == NULL) { return NULL; + } PyModule_AddObject(m, "times_result", (PyObject *)TimesResultType); uname_result_desc.name = MODNAME ".uname_result"; UnameResultType = PyStructSequence_NewType(&uname_result_desc); - if (UnameResultType == NULL) + if (UnameResultType == NULL) { return NULL; + } PyModule_AddObject(m, "uname_result", (PyObject *)UnameResultType); #ifdef __APPLE__ diff --git a/Objects/structseq.c b/Objects/structseq.c index 922928f9a3b2d9..d5137109f8db57 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -363,8 +363,8 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) /* PyTypeObject has already been initialized */ if (Py_REFCNT(type) != 0) { - PyErr_BadInternalCall(); - return -1; + PyErr_BadInternalCall(); + return -1; } type->tp_name = desc->name;