[New-bugs-announce] [issue16404] Uses of PyLong_FromLong that don't check for errors

[Ned Batchelder]

New submission from Ned Batchelder:

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);
   }

----------
messages: 174809
nosy: nedbat
priority: normal
severity: normal
status: open
title: Uses of PyLong_FromLong that don't check for errors
versions: Python 2.7, Python 3.4

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue16404>
_______________________________________