From dc65d661718ed10a9d212f1949813f7a7acf9437 Mon Sep 17 00:00:00 2001 From: Sebastian Koslowski Date: Sun, 24 Nov 2019 08:33:05 +0100 Subject: [PATCH] support for readonly buffers (#863) (#1466) --- include/pybind11/buffer_info.h | 28 +++++++++++++++++----------- include/pybind11/detail/class.h | 7 +++++++ include/pybind11/pytypes.h | 2 +- tests/constructor_stats.h | 2 +- tests/test_buffers.cpp | 26 ++++++++++++++++++++++++++ tests/test_buffers.py | 31 +++++++++++++++++++++++++++++++ 6 files changed, 83 insertions(+), 13 deletions(-) diff --git a/include/pybind11/buffer_info.h b/include/pybind11/buffer_info.h index b106d2cc6..1f4115a1f 100644 --- a/include/pybind11/buffer_info.h +++ b/include/pybind11/buffer_info.h @@ -22,13 +22,14 @@ struct buffer_info { ssize_t ndim = 0; // Number of dimensions std::vector shape; // Shape of the tensor (1 entry per dimension) std::vector strides; // Number of bytes between adjacent entries (for each per dimension) + bool readonly = false; // flag to indicate if the underlying storage may be written to buffer_info() { } buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t ndim, - detail::any_container shape_in, detail::any_container strides_in) + detail::any_container shape_in, detail::any_container strides_in, bool readonly=false) : ptr(ptr), itemsize(itemsize), size(1), format(format), ndim(ndim), - shape(std::move(shape_in)), strides(std::move(strides_in)) { + shape(std::move(shape_in)), strides(std::move(strides_in)), readonly(readonly) { if (ndim != (ssize_t) shape.size() || ndim != (ssize_t) strides.size()) pybind11_fail("buffer_info: ndim doesn't match shape and/or strides length"); for (size_t i = 0; i < (size_t) ndim; ++i) @@ -36,19 +37,23 @@ struct buffer_info { } template - buffer_info(T *ptr, detail::any_container shape_in, detail::any_container strides_in) - : buffer_info(private_ctr_tag(), ptr, sizeof(T), format_descriptor::format(), static_cast(shape_in->size()), std::move(shape_in), std::move(strides_in)) { } + buffer_info(T *ptr, detail::any_container shape_in, detail::any_container strides_in, bool readonly=false) + : buffer_info(private_ctr_tag(), ptr, sizeof(T), format_descriptor::format(), static_cast(shape_in->size()), std::move(shape_in), std::move(strides_in), readonly) { } - buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t size) - : buffer_info(ptr, itemsize, format, 1, {size}, {itemsize}) { } + buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t size, bool readonly=false) + : buffer_info(ptr, itemsize, format, 1, {size}, {itemsize}, readonly) { } template - buffer_info(T *ptr, ssize_t size) - : buffer_info(ptr, sizeof(T), format_descriptor::format(), size) { } + buffer_info(T *ptr, ssize_t size, bool readonly=false) + : buffer_info(ptr, sizeof(T), format_descriptor::format(), size, readonly) { } + + template + buffer_info(const T *ptr, ssize_t size, bool readonly=true) + : buffer_info(const_cast(ptr), sizeof(T), format_descriptor::format(), size, readonly) { } explicit buffer_info(Py_buffer *view, bool ownview = true) : buffer_info(view->buf, view->itemsize, view->format, view->ndim, - {view->shape, view->shape + view->ndim}, {view->strides, view->strides + view->ndim}) { + {view->shape, view->shape + view->ndim}, {view->strides, view->strides + view->ndim}, view->readonly) { this->view = view; this->ownview = ownview; } @@ -70,6 +75,7 @@ struct buffer_info { strides = std::move(rhs.strides); std::swap(view, rhs.view); std::swap(ownview, rhs.ownview); + readonly = rhs.readonly; return *this; } @@ -81,8 +87,8 @@ private: struct private_ctr_tag { }; buffer_info(private_ctr_tag, void *ptr, ssize_t itemsize, const std::string &format, ssize_t ndim, - detail::any_container &&shape_in, detail::any_container &&strides_in) - : buffer_info(ptr, itemsize, format, ndim, std::move(shape_in), std::move(strides_in)) { } + detail::any_container &&shape_in, detail::any_container &&strides_in, bool readonly) + : buffer_info(ptr, itemsize, format, ndim, std::move(shape_in), std::move(strides_in), readonly) { } Py_buffer *view = nullptr; bool ownview = false; diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 230ae81ae..edfa7de68 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -491,6 +491,13 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla view->len = view->itemsize; for (auto s : info->shape) view->len *= s; + view->readonly = info->readonly; + if ((flags & PyBUF_WRITABLE) == PyBUF_WRITABLE && info->readonly) { + if (view) + view->obj = nullptr; + PyErr_SetString(PyExc_BufferError, "Writable buffer requested for readonly storage"); + return -1; + } if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT) view->format = const_cast(info->format.c_str()); if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) { diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 96eab9662..4003d6918 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1345,7 +1345,7 @@ public: buf.strides = py_strides.data(); buf.shape = py_shape.data(); buf.suboffsets = nullptr; - buf.readonly = false; + buf.readonly = info.readonly; buf.internal = nullptr; m_ptr = PyMemoryView_FromBuffer(&buf); diff --git a/tests/constructor_stats.h b/tests/constructor_stats.h index f026e70f9..431e5acef 100644 --- a/tests/constructor_stats.h +++ b/tests/constructor_stats.h @@ -180,7 +180,7 @@ public: } } } - catch (const std::out_of_range &) {} + catch (const std::out_of_range&) {} if (!t1) throw std::runtime_error("Unknown class passed to ConstructorStats::get()"); auto &cs1 = get(*t1); // If we have both a t1 and t2 match, one is probably the trampoline class; return whichever diff --git a/tests/test_buffers.cpp b/tests/test_buffers.cpp index 433dfeee6..1bc67ff7b 100644 --- a/tests/test_buffers.cpp +++ b/tests/test_buffers.cpp @@ -166,4 +166,30 @@ TEST_SUBMODULE(buffers, m) { .def_readwrite("value", (int32_t DerivedBuffer::*) &DerivedBuffer::value) .def_buffer(&DerivedBuffer::get_buffer_info); + struct BufferReadOnly { + const uint8_t value = 0; + BufferReadOnly(uint8_t value): value(value) {} + + py::buffer_info get_buffer_info() { + return py::buffer_info(&value, 1); + } + }; + py::class_(m, "BufferReadOnly", py::buffer_protocol()) + .def(py::init()) + .def_buffer(&BufferReadOnly::get_buffer_info); + + struct BufferReadOnlySelect { + uint8_t value = 0; + bool readonly = false; + + py::buffer_info get_buffer_info() { + return py::buffer_info(&value, 1, readonly); + } + }; + py::class_(m, "BufferReadOnlySelect", py::buffer_protocol()) + .def(py::init<>()) + .def_readwrite("value", &BufferReadOnlySelect::value) + .def_readwrite("readonly", &BufferReadOnlySelect::readonly) + .def_buffer(&BufferReadOnlySelect::get_buffer_info); + } diff --git a/tests/test_buffers.py b/tests/test_buffers.py index f006552bf..bf7aaed70 100644 --- a/tests/test_buffers.py +++ b/tests/test_buffers.py @@ -1,8 +1,14 @@ +import io import struct +import sys + import pytest + from pybind11_tests import buffers as m from pybind11_tests import ConstructorStats +PY3 = sys.version_info[0] >= 3 + pytestmark = pytest.requires_numpy with pytest.suppress(ImportError): @@ -85,3 +91,28 @@ def test_pointer_to_member_fn(): buf.value = 0x12345678 value = struct.unpack('i', bytearray(buf))[0] assert value == 0x12345678 + + +@pytest.unsupported_on_pypy +def test_readonly_buffer(): + buf = m.BufferReadOnly(0x64) + view = memoryview(buf) + assert view[0] == 0x64 if PY3 else b'd' + assert view.readonly + + +@pytest.unsupported_on_pypy +def test_selective_readonly_buffer(): + buf = m.BufferReadOnlySelect() + + memoryview(buf)[0] = 0x64 if PY3 else b'd' + assert buf.value == 0x64 + + io.BytesIO(b'A').readinto(buf) + assert buf.value == ord(b'A') + + buf.readonly = True + with pytest.raises(TypeError): + memoryview(buf)[0] = 0 if PY3 else b'\0' + with pytest.raises(TypeError): + io.BytesIO(b'1').readinto(buf)