BufferProxy: IndexError exception thrown sporadically from _bufferproxy_write

Issue #109 resolved
lifning created an issue

The call to PyArg_ParseTuple in _bufferproxy_write uses the wrong formatting string to fill &offset. It uses "i" which is for int, rather than "n" which is for Py_ssize_t. http://docs.python.org/c-api/arg.html

sizeof(Py_ssize_t) on x86_64 is 8, whereas sizeof(int) is 4. If PyArg_ParseTuple is given an "i" it will fill the least-significant (on little-endian) four bytes of the variable, leaving the most-significant untouched. This bug probably wasn't visible on 32-bit x86, where they are both 4 bytes wide.

The end result of this is that _bufferproxy_write ends up throwing a Python IndexError exception sporadically (whenever the uninitialized bytes are not zero) when the arguments passed are otherwise valid.

To reproduce on an x86_64 machine, call it repeatedly until some non-zero garbage value happens to occupy the more significant bytes of offset when _bufferproxy_write is called. {{{ s = some_surface_or_sound data = some_string_that_should_fit_in_the_buffer encountered = False while not encountered: try: s.get_buffer.write(data, 0) except IndexError: encountered = True }}} A fix is attached.

Comments (12)

  1. Lenard Lindstrom

    If we are no longer longer concerned with Python 2.4 support then the patch will be applied as is.

  2. lifning reporter

    If 2.4 support ends up being an issue, a slightly more hackish fix that should be compatible would be to initialize that variable to 0 where it is declared.

  3. cgohlke

    How about the proposed change to src/color.c? I have not looked at it in detail, but Py_ssize_t might be unnecessary for a value in range(1, 5). In any case, the current code needs attention for x64.

  4. Log in to comment