[New-bugs-announce] [issue43608] `bytes_concat` and Buffer cleanup

[Sebastian Berg]

New submission from Sebastian Berg <sebastian at sipsolutions.net>:

`pybytes_concate` currently uses the following code to get the data:

    va.len = -1;
    vb.len = -1;
    if (PyObject_GetBuffer(a, &va, PyBUF_SIMPLE) != 0 ||
        PyObject_GetBuffer(b, &vb, PyBUF_SIMPLE) != 0) {
        PyErr_Format(PyExc_TypeError, "can't concat %.100s to %.100s",
                     Py_TYPE(b)->tp_name, Py_TYPE(a)->tp_name);
        goto done;
    }

I don't actually know if it is realistically possible to issues here (I ended up here by chasing the wrong thing).  But this (and the identical code in `bytearray`) strictly rely on `view->len` not being modified on error (or else may not clean `va`)!

That seems wrong to me?  Although, I now saw that `PyBuffer_GetBuffer` says:

    If the exporter cannot provide a buffer of the exact type, it MUST raise PyExc_BufferError, set view->obj to NULL and return -1.

Pretty much all code in NumPy (and cpython as far as I can tell), will guarantee that `obj` (and `len` probably) is untouched on error, but it will not set it to NULL!

I can see some wisdom in NULL'ing `view->obj` since it means the caller can call `PyBuffer_Release` unconditionally (but then we have to explicitly do that!).  But realistically, it seems to me the realistic thing is to say that a caller must never release an unexported buffer and make no assumption about its content?

(Which doesn't mean I won't ensure NumPy will keep `len` and `obj` unmodified or NULL `obj` on error.)

----------
components: C API
messages: 389428
nosy: seberg
priority: normal
severity: normal
status: open
title: `bytes_concat` and Buffer cleanup
versions: Python 3.10, Python 3.9

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue43608>
_______________________________________