classification
Title: Uses of PyLong_FromLong that don't check for errors
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: asvetlov, docs@python, jcea, mark.dickinson, nedbat, python-dev, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2012-11-04 14:57 by nedbat, last changed 2013-12-17 21:21 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
issue16404.patch serhiy.storchaka, 2013-11-30 15:40 review
Messages (4)
msg174809 - (view) Author: Ned Batchelder (nedbat) * (Python triager) Date: 2012-11-04 14:57
Examining the CPython sources, there are a number of places where PyLong_FromLong is used without checking its return value. In places where it is done correctly, PyErr_Occurred is often used to avoid having to check every call.

Here are places where it isn't done properly.  I can make patches if desired, though I can't build and test on Windows.  This code is from CPython tip, but the same issues exist in the 2.7 branch.

In Modules/arraymodule.c:

static PyObject *
array_buffer_info(arrayobject *self, PyObject *unused)
{
    PyObject* retval = NULL;
    retval = PyTuple_New(2);
    if (!retval)
        return NULL;

    PyTuple_SET_ITEM(retval, 0, PyLong_FromVoidPtr(self->ob_item));
    PyTuple_SET_ITEM(retval, 1, PyLong_FromLong((long)(Py_SIZE(self))));

    return retval;
}

In Modules/ossaudiodev.c

    /* Construct the return value: a (fmt, channels, rate) tuple that
       tells what the audio hardware was actually set to. */
    rv = PyTuple_New(3);
    if (rv == NULL)
        return NULL;
    PyTuple_SET_ITEM(rv, 0, PyLong_FromLong(fmt));
    PyTuple_SET_ITEM(rv, 1, PyLong_FromLong(channels));
    PyTuple_SET_ITEM(rv, 2, PyLong_FromLong(rate));
    return rv;

These 7 lines could be replaced simply with:

    return Py_BuildValue("(iii)", fmt, channels, rate);


In Python/sysmodule.c, where the PyUnicode_FromString is far more worrisome:

static PyObject *
sys_getwindowsversion(PyObject *self)
{
    PyObject *version;
    int pos = 0;
    OSVERSIONINFOEX ver;
    ver.dwOSVersionInfoSize = sizeof(ver);
    if (!GetVersionEx((OSVERSIONINFO*) &ver))
        return PyErr_SetFromWindowsErr(0);

    version = PyStructSequence_New(&WindowsVersionType);
    if (version == NULL)
        return NULL;

    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwMajorVersion));
    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwMinorVersion));
    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwBuildNumber));
    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwPlatformId));
    PyStructSequence_SET_ITEM(version, pos++, PyUnicode_FromString(ver.szCSDVersion));
    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wServicePackMajor));
    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wServicePackMinor));
    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wSuiteMask));
    PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wProductType));

    return version;
}

In Doc/extending/extending.rst, as an example of a correct function!:

   void
   no_bug(PyObject *list)
   {
       PyObject *item = PyList_GetItem(list, 0);

       Py_INCREF(item);
       PyList_SetItem(list, 1, PyLong_FromLong(0L));
       PyObject_Print(item, stdout, 0);
       Py_DECREF(item);
   }
msg174817 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-04 16:10
See also issue15989.
msg204818 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-11-30 15:16
Here is a patch. arraymodule.c is already fixed. Instead I found other bug in sysmodule.c. I'm not sure about extending.rst, PyLong_FromLong(0L) should never fail if NSMALLNEGINTS + NSMALLPOSINTS > 0.
msg206432 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-12-17 13:15
New changeset debdfa44f020 by Serhiy Storchaka in branch '2.7':
Issue #16404: Add checks for return value of PyInt_FromLong() in
http://hg.python.org/cpython/rev/debdfa44f020

New changeset 928c0acf7c09 by Serhiy Storchaka in branch '3.3':
Issue #16404: Add checks for return value of PyLong_FromLong() in
http://hg.python.org/cpython/rev/928c0acf7c09

New changeset d489394a73de by Serhiy Storchaka in branch 'default':
Issue #16404: Add checks for return value of PyLong_FromLong() in
http://hg.python.org/cpython/rev/d489394a73de
History
Date User Action Args
2013-12-17 21:21:52serhiy.storchakasetstatus: open -> closed
2013-12-17 13:17:54serhiy.storchakasetresolution: fixed
stage: patch review -> resolved
2013-12-17 13:15:18python-devsetnosy: + python-dev
messages: + msg206432
2013-12-02 11:08:48vstinnersetnosy: + vstinner
2013-11-30 15:40:15serhiy.storchakasetfiles: + issue16404.patch
components: - Documentation, Interpreter Core
2013-11-30 15:39:26serhiy.storchakasetfiles: - issue16404.patch
2013-11-30 15:16:50serhiy.storchakasetfiles: + issue16404.patch
versions: - Python 3.2
messages: + msg204818

keywords: + patch
stage: needs patch -> patch review
2013-01-07 18:39:23serhiy.storchakasetassignee: docs@python -> serhiy.storchaka
2012-11-15 15:46:01asvetlovsetnosy: + asvetlov
2012-11-05 17:12:47jceasetnosy: + jcea
2012-11-04 16:10:37serhiy.storchakasetassignee: docs@python
type: behavior
components: + Documentation, Extension Modules, Interpreter Core
versions: + Python 3.2, Python 3.3
nosy: + mark.dickinson, docs@python, serhiy.storchaka

messages: + msg174817
stage: needs patch
2012-11-04 14:57:07nedbatcreate