[Python-Dev] [Python-checkins] cpython: Issue #18520: Add a new PyStructSequence_InitType2() function, same than
Ronald Oussoren
ronaldoussoren at mac.com
Tue Jul 23 08:15:29 CEST 2013
On 23 Jul, 2013, at 2:01, Benjamin Peterson <benjamin at python.org> wrote:
> We've cheerfully broken the ABI before on minor releases, though if
> it's part of the stable ABI, we can't be cavaliar about that anymore.
It is not part of the stable ABI. Given that the implementation of
PyStructSequence_InitType() in the patch just calls PyStructSequence_InitType2()
and ignores the return value you could change the return value of ..InitType().
This may or may not break existing extensions using the function (depending on
platform ABI details, AFAIK this is not a problem on x86/x86_64), but reusing
extensions across python feature releases is not supported anyway. There are
no problems when compiling code, most C compilers won't even warn about ignored
return values unless you explicitly ask for it.
Ronald
>
> 2013/7/22 Victor Stinner <victor.stinner at gmail.com>:
>> "Add a new PyStructSequence_InitType2()"
>>
>> I added a new function because I guess that it would break the API (and ABI)
>> to change the return type of a function in a minor release.
>>
>> Tell me if you have a better name than PyStructSequence_InitType2() ;-)
>>
>> "Ex" suffix is usually used when parameters are added. It is not the case
>> here.
>>
>> Victor
>>
>> Le 22 juil. 2013 23:59, "victor.stinner" <python-checkins at python.org> a
>> écrit :
>>>
>>> http://hg.python.org/cpython/rev/fc718c177ee6
>>> changeset: 84793:fc718c177ee6
>>> user: Victor Stinner <victor.stinner at gmail.com>
>>> date: Mon Jul 22 22:24:54 2013 +0200
>>> summary:
>>> Issue #18520: Add a new PyStructSequence_InitType2() function, same than
>>> PyStructSequence_InitType() except that it has a return value (0 on
>>> success,
>>> -1 on error).
>>>
>>> * PyStructSequence_InitType2() now raises MemoryError on memory
>>> allocation failure
>>> * Fix also some calls to PyDict_SetItemString(): handle error
>>>
>>> files:
>>> Include/pythonrun.h | 2 +-
>>> Include/structseq.h | 2 +
>>> Misc/NEWS | 4 +++
>>> Modules/_lsprof.c | 10 ++++---
>>> Modules/grpmodule.c | 11 ++++++--
>>> Modules/posixmodule.c | 24 ++++++++++++------
>>> Modules/pwdmodule.c | 5 ++-
>>> Modules/resource.c | 9 ++++--
>>> Modules/signalmodule.c | 7 +++--
>>> Modules/spwdmodule.c | 8 ++++--
>>> Modules/timemodule.c | 5 ++-
>>> Objects/floatobject.c | 9 ++++--
>>> Objects/longobject.c | 6 +++-
>>> Objects/structseq.c | 37 +++++++++++++++++++++--------
>>> Python/pythonrun.c | 3 +-
>>> Python/sysmodule.c | 23 ++++++++++++-----
>>> Python/thread.c | 6 +++-
>>> 17 files changed, 117 insertions(+), 54 deletions(-)
>>>
>>>
>>> diff --git a/Include/pythonrun.h b/Include/pythonrun.h
>>> --- a/Include/pythonrun.h
>>> +++ b/Include/pythonrun.h
>>> @@ -197,7 +197,7 @@
>>> PyAPI_FUNC(void) _PyExc_Init(PyObject * bltinmod);
>>> PyAPI_FUNC(void) _PyImportHooks_Init(void);
>>> PyAPI_FUNC(int) _PyFrame_Init(void);
>>> -PyAPI_FUNC(void) _PyFloat_Init(void);
>>> +PyAPI_FUNC(int) _PyFloat_Init(void);
>>> PyAPI_FUNC(int) PyByteArray_Init(void);
>>> PyAPI_FUNC(void) _PyRandom_Init(void);
>>> #endif
>>> diff --git a/Include/structseq.h b/Include/structseq.h
>>> --- a/Include/structseq.h
>>> +++ b/Include/structseq.h
>>> @@ -24,6 +24,8 @@
>>> #ifndef Py_LIMITED_API
>>> PyAPI_FUNC(void) PyStructSequence_InitType(PyTypeObject *type,
>>> PyStructSequence_Desc *desc);
>>> +PyAPI_FUNC(int) PyStructSequence_InitType2(PyTypeObject *type,
>>> + PyStructSequence_Desc *desc);
>>> #endif
>>> PyAPI_FUNC(PyTypeObject*) PyStructSequence_NewType(PyStructSequence_Desc
>>> *desc);
>>>
>>> diff --git a/Misc/NEWS b/Misc/NEWS
>>> --- a/Misc/NEWS
>>> +++ b/Misc/NEWS
>>> @@ -10,6 +10,10 @@
>>> Core and Builtins
>>> -----------------
>>>
>>> +- Issue #18520: Add a new PyStructSequence_InitType2() function, same
>>> than
>>> + PyStructSequence_InitType() except that it has a return value (0 on
>>> success,
>>> + -1 on error).
>>> +
>>> - Issue #15905: Fix theoretical buffer overflow in handling of
>>> sys.argv[0],
>>> prefix and exec_prefix if the operation system does not obey
>>> MAXPATHLEN.
>>>
>>> diff --git a/Modules/_lsprof.c b/Modules/_lsprof.c
>>> --- a/Modules/_lsprof.c
>>> +++ b/Modules/_lsprof.c
>>> @@ -884,10 +884,12 @@
>>> PyDict_SetItemString(d, "Profiler", (PyObject *)&PyProfiler_Type);
>>>
>>> if (!initialized) {
>>> - PyStructSequence_InitType(&StatsEntryType,
>>> - &profiler_entry_desc);
>>> - PyStructSequence_InitType(&StatsSubEntryType,
>>> - &profiler_subentry_desc);
>>> + if (PyStructSequence_InitType2(&StatsEntryType,
>>> + &profiler_entry_desc) < 0)
>>> + return NULL;
>>> + if (PyStructSequence_InitType2(&StatsSubEntryType,
>>> + &profiler_subentry_desc) < 0)
>>> + return NULL;
>>> }
>>> Py_INCREF((PyObject*) &StatsEntryType);
>>> Py_INCREF((PyObject*) &StatsSubEntryType);
>>> diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c
>>> --- a/Modules/grpmodule.c
>>> +++ b/Modules/grpmodule.c
>>> @@ -210,9 +210,14 @@
>>> if (m == NULL)
>>> return NULL;
>>> d = PyModule_GetDict(m);
>>> - if (!initialized)
>>> - PyStructSequence_InitType(&StructGrpType,
>>> &struct_group_type_desc);
>>> - PyDict_SetItemString(d, "struct_group", (PyObject *) &StructGrpType);
>>> + if (!initialized) {
>>> + if (PyStructSequence_InitType2(&StructGrpType,
>>> + &struct_group_type_desc) < 0)
>>> + return NULL;
>>> + }
>>> + if (PyDict_SetItemString(d, "struct_group",
>>> + (PyObject *)&StructGrpType) < 0)
>>> + return NULL;
>>> initialized = 1;
>>> return m;
>>> }
>>> diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
>>> --- a/Modules/posixmodule.c
>>> +++ b/Modules/posixmodule.c
>>> @@ -11518,19 +11518,23 @@
>>> if (!initialized) {
>>> #if defined(HAVE_WAITID) && !defined(__APPLE__)
>>> waitid_result_desc.name = MODNAME ".waitid_result";
>>> - PyStructSequence_InitType(&WaitidResultType,
>>> &waitid_result_desc);
>>> + if (PyStructSequence_InitType2(&WaitidResultType,
>>> &waitid_result_desc) < 0)
>>> + return NULL;
>>> #endif
>>>
>>> stat_result_desc.name = MODNAME ".stat_result";
>>> stat_result_desc.fields[7].name = PyStructSequence_UnnamedField;
>>> stat_result_desc.fields[8].name = PyStructSequence_UnnamedField;
>>> stat_result_desc.fields[9].name = PyStructSequence_UnnamedField;
>>> - PyStructSequence_InitType(&StatResultType, &stat_result_desc);
>>> + if (PyStructSequence_InitType2(&StatResultType,
>>> &stat_result_desc) < 0)
>>> + return NULL;
>>> structseq_new = StatResultType.tp_new;
>>> StatResultType.tp_new = statresult_new;
>>>
>>> statvfs_result_desc.name = MODNAME ".statvfs_result";
>>> - PyStructSequence_InitType(&StatVFSResultType,
>>> &statvfs_result_desc);
>>> + if (PyStructSequence_InitType2(&StatVFSResultType,
>>> + &statvfs_result_desc) < 0)
>>> + return NULL;
>>> #ifdef NEED_TICKS_PER_SECOND
>>> # if defined(HAVE_SYSCONF) && defined(_SC_CLK_TCK)
>>> ticks_per_second = sysconf(_SC_CLK_TCK);
>>> @@ -11543,12 +11547,15 @@
>>>
>>> #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER)
>>> sched_param_desc.name = MODNAME ".sched_param";
>>> - PyStructSequence_InitType(&SchedParamType, &sched_param_desc);
>>> + if (PyStructSequence_InitType2(&SchedParamType,
>>> &sched_param_desc) < 0)
>>> + return NULL;
>>> SchedParamType.tp_new = sched_param_new;
>>> #endif
>>>
>>> /* initialize TerminalSize_info */
>>> - PyStructSequence_InitType(&TerminalSizeType, &TerminalSize_desc);
>>> + if (PyStructSequence_InitType2(&TerminalSizeType,
>>> + &TerminalSize_desc) < 0)
>>> + return NULL;
>>> }
>>> #if defined(HAVE_WAITID) && !defined(__APPLE__)
>>> Py_INCREF((PyObject*) &WaitidResultType);
>>> @@ -11566,11 +11573,13 @@
>>> #endif
>>>
>>> times_result_desc.name = MODNAME ".times_result";
>>> - PyStructSequence_InitType(&TimesResultType, ×_result_desc);
>>> + if (PyStructSequence_InitType2(&TimesResultType, ×_result_desc)
>>> < 0)
>>> + return NULL;
>>> PyModule_AddObject(m, "times_result", (PyObject *)&TimesResultType);
>>>
>>> uname_result_desc.name = MODNAME ".uname_result";
>>> - PyStructSequence_InitType(&UnameResultType, &uname_result_desc);
>>> + if (PyStructSequence_InitType2(&UnameResultType, &uname_result_desc)
>>> < 0)
>>> + return NULL;
>>> PyModule_AddObject(m, "uname_result", (PyObject *)&UnameResultType);
>>>
>>> #ifdef __APPLE__
>>> @@ -11648,7 +11657,6 @@
>>> initialized = 1;
>>>
>>> return m;
>>> -
>>> }
>>>
>>> #ifdef __cplusplus
>>> diff --git a/Modules/pwdmodule.c b/Modules/pwdmodule.c
>>> --- a/Modules/pwdmodule.c
>>> +++ b/Modules/pwdmodule.c
>>> @@ -216,8 +216,9 @@
>>> return NULL;
>>>
>>> if (!initialized) {
>>> - PyStructSequence_InitType(&StructPwdType,
>>> - &struct_pwd_type_desc);
>>> + if (PyStructSequence_InitType2(&StructPwdType,
>>> + &struct_pwd_type_desc) < 0)
>>> + return NULL;
>>> initialized = 1;
>>> }
>>> Py_INCREF((PyObject *) &StructPwdType);
>>> diff --git a/Modules/resource.c b/Modules/resource.c
>>> --- a/Modules/resource.c
>>> +++ b/Modules/resource.c
>>> @@ -263,9 +263,12 @@
>>> /* Add some symbolic constants to the module */
>>> Py_INCREF(PyExc_OSError);
>>> PyModule_AddObject(m, "error", PyExc_OSError);
>>> - if (!initialized)
>>> - PyStructSequence_InitType(&StructRUsageType,
>>> - &struct_rusage_desc);
>>> + if (!initialized) {
>>> + if (PyStructSequence_InitType2(&StructRUsageType,
>>> + &struct_rusage_desc) < 0)
>>> + return NULL;
>>> + }
>>> +
>>> Py_INCREF(&StructRUsageType);
>>> PyModule_AddObject(m, "struct_rusage",
>>> (PyObject*) &StructRUsageType);
>>> diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
>>> --- a/Modules/signalmodule.c
>>> +++ b/Modules/signalmodule.c
>>> @@ -978,9 +978,10 @@
>>> return NULL;
>>>
>>> #if defined(HAVE_SIGWAITINFO) || defined(HAVE_SIGTIMEDWAIT)
>>> - if (!initialized)
>>> - PyStructSequence_InitType(&SiginfoType, &struct_siginfo_desc);
>>> -
>>> + if (!initialized) {
>>> + if (PyStructSequence_InitType2(&SiginfoType,
>>> &struct_siginfo_desc) < 0)
>>> + return NULL;
>>> + }
>>> Py_INCREF((PyObject*) &SiginfoType);
>>> PyModule_AddObject(m, "struct_siginfo", (PyObject*) &SiginfoType);
>>> initialized = 1;
>>> diff --git a/Modules/spwdmodule.c b/Modules/spwdmodule.c
>>> --- a/Modules/spwdmodule.c
>>> +++ b/Modules/spwdmodule.c
>>> @@ -196,9 +196,11 @@
>>> m=PyModule_Create(&spwdmodule);
>>> if (m == NULL)
>>> return NULL;
>>> - if (!initialized)
>>> - PyStructSequence_InitType(&StructSpwdType,
>>> - &struct_spwd_type_desc);
>>> + if (!initialized) {
>>> + if (PyStructSequence_InitType2(&StructSpwdType,
>>> + &struct_spwd_type_desc) < 0)
>>> + return NULL;
>>> + }
>>> Py_INCREF((PyObject *) &StructSpwdType);
>>> PyModule_AddObject(m, "struct_spwd", (PyObject *) &StructSpwdType);
>>> initialized = 1;
>>> diff --git a/Modules/timemodule.c b/Modules/timemodule.c
>>> --- a/Modules/timemodule.c
>>> +++ b/Modules/timemodule.c
>>> @@ -1476,8 +1476,9 @@
>>> PyInit_timezone(m);
>>>
>>> if (!initialized) {
>>> - PyStructSequence_InitType(&StructTimeType,
>>> - &struct_time_type_desc);
>>> + if (PyStructSequence_InitType2(&StructTimeType,
>>> + &struct_time_type_desc) < 0)
>>> + return NULL;
>>>
>>> #ifdef MS_WINDOWS
>>> winver.dwOSVersionInfoSize = sizeof(winver);
>>> diff --git a/Objects/floatobject.c b/Objects/floatobject.c
>>> --- a/Objects/floatobject.c
>>> +++ b/Objects/floatobject.c
>>> @@ -1853,7 +1853,7 @@
>>> float_new, /* tp_new */
>>> };
>>>
>>> -void
>>> +int
>>> _PyFloat_Init(void)
>>> {
>>> /* We attempt to determine if this machine is using IEEE
>>> @@ -1903,8 +1903,11 @@
>>> float_format = detected_float_format;
>>>
>>> /* Init float info */
>>> - if (FloatInfoType.tp_name == 0)
>>> - PyStructSequence_InitType(&FloatInfoType, &floatinfo_desc);
>>> + if (FloatInfoType.tp_name == NULL) {
>>> + if (PyStructSequence_InitType2(&FloatInfoType, &floatinfo_desc) <
>>> 0)
>>> + return 0;
>>> + }
>>> + return 1;
>>> }
>>>
>>> int
>>> diff --git a/Objects/longobject.c b/Objects/longobject.c
>>> --- a/Objects/longobject.c
>>> +++ b/Objects/longobject.c
>>> @@ -5059,8 +5059,10 @@
>>> }
>>> #endif
>>> /* initialize int_info */
>>> - if (Int_InfoType.tp_name == 0)
>>> - PyStructSequence_InitType(&Int_InfoType, &int_info_desc);
>>> + if (Int_InfoType.tp_name == NULL) {
>>> + if (PyStructSequence_InitType2(&Int_InfoType, &int_info_desc) <
>>> 0)
>>> + return 0;
>>> + }
>>>
>>> return 1;
>>> }
>>> diff --git a/Objects/structseq.c b/Objects/structseq.c
>>> --- a/Objects/structseq.c
>>> +++ b/Objects/structseq.c
>>> @@ -320,12 +320,13 @@
>>> structseq_new, /* tp_new */
>>> };
>>>
>>> -void
>>> -PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc
>>> *desc)
>>> +int
>>> +PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc
>>> *desc)
>>> {
>>> PyObject *dict;
>>> PyMemberDef* members;
>>> int n_members, n_unnamed_members, i, k;
>>> + PyObject *v;
>>>
>>> #ifdef Py_TRACE_REFS
>>> /* if the type object was chained, unchain it first
>>> @@ -347,8 +348,10 @@
>>> type->tp_doc = desc->doc;
>>>
>>> members = PyMem_NEW(PyMemberDef, n_members-n_unnamed_members+1);
>>> - if (members == NULL)
>>> - return;
>>> + if (members == NULL) {
>>> + PyErr_NoMemory();
>>> + return -1;
>>> + }
>>>
>>> for (i = k = 0; i < n_members; ++i) {
>>> if (desc->fields[i].name == PyStructSequence_UnnamedField)
>>> @@ -366,22 +369,33 @@
>>> type->tp_members = members;
>>>
>>> if (PyType_Ready(type) < 0)
>>> - return;
>>> + return -1;
>>> Py_INCREF(type);
>>>
>>> dict = type->tp_dict;
>>> #define SET_DICT_FROM_INT(key, value) \
>>> do { \
>>> - PyObject *v = PyLong_FromLong((long) value); \
>>> - if (v != NULL) { \
>>> - PyDict_SetItemString(dict, key, v); \
>>> + v = PyLong_FromLong((long) 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_INT(visible_length_key, desc->n_in_sequence);
>>> SET_DICT_FROM_INT(real_length_key, n_members);
>>> SET_DICT_FROM_INT(unnamed_fields_key, n_unnamed_members);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void
>>> +PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc
>>> *desc)
>>> +{
>>> + (void)PyStructSequence_InitType2(type, desc);
>>> }
>>>
>>> PyTypeObject*
>>> @@ -390,8 +404,11 @@
>>> PyTypeObject *result;
>>>
>>> result = (PyTypeObject*)PyType_GenericAlloc(&PyType_Type, 0);
>>> - if (result != NULL) {
>>> - PyStructSequence_InitType(result, desc);
>>> + if (result == NULL)
>>> + return NULL;
>>> + if (PyStructSequence_InitType2(result, desc) < 0) {
>>> + Py_DECREF(result);
>>> + return NULL;
>>> }
>>> return result;
>>> }
>>> diff --git a/Python/pythonrun.c b/Python/pythonrun.c
>>> --- a/Python/pythonrun.c
>>> +++ b/Python/pythonrun.c
>>> @@ -328,7 +328,8 @@
>>> if (!PyByteArray_Init())
>>> Py_FatalError("Py_Initialize: can't init bytearray");
>>>
>>> - _PyFloat_Init();
>>> + if (!_PyFloat_Init())
>>> + Py_FatalError("Py_Initialize: can't init float");
>>>
>>> interp->modules = PyDict_New();
>>> if (interp->modules == NULL)
>>> diff --git a/Python/sysmodule.c b/Python/sysmodule.c
>>> --- a/Python/sysmodule.c
>>> +++ b/Python/sysmodule.c
>>> @@ -1634,8 +1634,10 @@
>>> SET_SYS_FROM_STRING("int_info",
>>> PyLong_GetInfo());
>>> /* initialize hash_info */
>>> - if (Hash_InfoType.tp_name == 0)
>>> - PyStructSequence_InitType(&Hash_InfoType, &hash_info_desc);
>>> + if (Hash_InfoType.tp_name == NULL) {
>>> + if (PyStructSequence_InitType2(&Hash_InfoType, &hash_info_desc) <
>>> 0)
>>> + return NULL;
>>> + }
>>> SET_SYS_FROM_STRING("hash_info",
>>> get_hash_info());
>>> SET_SYS_FROM_STRING("maxunicode",
>>> @@ -1676,8 +1678,11 @@
>>> }
>>>
>>> /* version_info */
>>> - if (VersionInfoType.tp_name == 0)
>>> - PyStructSequence_InitType(&VersionInfoType, &version_info_desc);
>>> + if (VersionInfoType.tp_name == NULL) {
>>> + if (PyStructSequence_InitType2(&VersionInfoType,
>>> + &version_info_desc) < 0)
>>> + return NULL;
>>> + }
>>> version_info = make_version_info();
>>> SET_SYS_FROM_STRING("version_info", version_info);
>>> /* prevent user from creating new instances */
>>> @@ -1688,8 +1693,10 @@
>>> SET_SYS_FROM_STRING("implementation", make_impl_info(version_info));
>>>
>>> /* flags */
>>> - if (FlagsType.tp_name == 0)
>>> - PyStructSequence_InitType(&FlagsType, &flags_desc);
>>> + if (FlagsType.tp_name == 0) {
>>> + if (PyStructSequence_InitType2(&FlagsType, &flags_desc) < 0)
>>> + return NULL;
>>> + }
>>> SET_SYS_FROM_STRING("flags", make_flags());
>>> /* prevent user from creating new instances */
>>> FlagsType.tp_init = NULL;
>>> @@ -1699,7 +1706,9 @@
>>> #if defined(MS_WINDOWS)
>>> /* getwindowsversion */
>>> if (WindowsVersionType.tp_name == 0)
>>> - PyStructSequence_InitType(&WindowsVersionType,
>>> &windows_version_desc);
>>> + if (PyStructSequence_InitType2(&WindowsVersionType,
>>> + &windows_version_desc) < 0)
>>> + return NULL;
>>> /* prevent user from creating new instances */
>>> WindowsVersionType.tp_init = NULL;
>>> WindowsVersionType.tp_new = NULL;
>>> diff --git a/Python/thread.c b/Python/thread.c
>>> --- a/Python/thread.c
>>> +++ b/Python/thread.c
>>> @@ -399,8 +399,10 @@
>>> int len;
>>> #endif
>>>
>>> - if (ThreadInfoType.tp_name == 0)
>>> - PyStructSequence_InitType(&ThreadInfoType, &threadinfo_desc);
>>> + if (ThreadInfoType.tp_name == 0) {
>>> + if (PyStructSequence_InitType2(&ThreadInfoType, &threadinfo_desc)
>>> < 0)
>>> + return NULL;
>>> + }
>>>
>>> threadinfo = PyStructSequence_New(&ThreadInfoType);
>>> if (threadinfo == NULL)
>>>
>>> --
>>> Repository URL: http://hg.python.org/cpython
>>>
>>> _______________________________________________
>>> Python-checkins mailing list
>>> Python-checkins at python.org
>>> http://mail.python.org/mailman/listinfo/python-checkins
>>>
>>
>> _______________________________________________
>> Python-Dev mailing list
>> Python-Dev at python.org
>> http://mail.python.org/mailman/listinfo/python-dev
>> Unsubscribe:
>> http://mail.python.org/mailman/options/python-dev/benjamin%40python.org
>>
>
>
>
> --
> Regards,
> Benjamin
> _______________________________________________
> Python-checkins mailing list
> Python-checkins at python.org
> http://mail.python.org/mailman/listinfo/python-checkins
More information about the Python-Dev
mailing list