Commits

Lenard Lindstrom  committed 11e8afc

Refactor the casting between the C (Py_buffer *) and (Pg_buffer *) types

Originally, a BufferProxy get buffer callback accepted a Pg_buffer * argument.
This was intended to protect against accidentally using such a callback as
a type's bf_releasebuffer slot function. But it left the code full of
downcasts from (Pg_buffer *) to (Py_buffer *), and hid the intent that any
bf_releasebuffer slot function can be used as a BufferProxy callback.
So the BufferProxy callback signature was changed to releasebufferproc.
A new PyBUF_PYGAME flag and C assertions are added to trace down callback
problems.

  • Participants
  • Parent commits 5e00ad8

Comments (0)

Files changed (5)

     pg_view_p->release_buffer = _release_buffer_generic;
     view_p->len = 0;
 
+#ifndef NDEBUG
+    /* Allow a callback to assert that it recieved a Pg_buffer,
+       not a Py_buffer */
+    flags |= PyBUF_PYGAME;
+#endif
+
 #if PG_ENABLE_NEWBUF
 
     if (PyObject_CheckBuffer (obj)) {

File src/bufferproxy.c

 
 typedef struct PgBufproxyObject_s {
     PyObject_HEAD
-    PyObject *obj;                             /* Wrapped object              */
-    Pg_buffer *view_p;                         /* For array interface export  */
+    PyObject *obj;                             /* Wrapped object (parent)     */
+    Pg_buffer *pg_view_p;                      /* For array interface export  */
 #if PG_ENABLE_OLDBUF
     Py_ssize_t segcount;                       /* bf_getsegcount return value */
     Py_ssize_t seglen;                         /* bf_getsegcount len argument */
 #endif
-    pg_getbufferfunc get_buffer;               /* Pg_buffer get callback      */
+    getbufferproc get_buffer;                  /* Pg_buffer get callback      */
     PyObject *dict;                            /* Allow arbitrary attributes  */
     PyObject *weakrefs;                        /* Reference cycles can happen */
 } PgBufproxyObject;
 #endif /* #if PY_VERSION_HEX < 0x02060000 */
 
 static int
-_get_buffer_from_dict(PyObject *dict, Pg_buffer *pg_view_p, int flags) {
+_get_buffer_from_dict(PyObject *dict, Py_buffer *view_p, int flags) {
     PyObject *obj;
-    Py_buffer *view_p = (Py_buffer *)pg_view_p;
     Pg_buffer *pg_dict_view_p;
     Py_buffer *dict_view_p;
     PyObject *py_callback;
         PyErr_NoMemory();
         return -1;
     }
-    pg_dict_view_p->consumer = pg_view_p->consumer;
+    pg_dict_view_p->consumer = ((Pg_buffer *)view_p)->consumer;
     if (PgDict_AsBuffer(pg_dict_view_p, dict, flags)) {
         PyMem_Free(pg_dict_view_p);
         return -1;
     view_p->strides = dict_view_p->strides;
     view_p->suboffsets = dict_view_p->suboffsets;
     view_p->internal = pg_dict_view_p;
-    pg_view_p->release_buffer = _release_buffer_from_dict;
+    ((Pg_buffer *)view_p)->release_buffer = _release_buffer_from_dict;
     return 0;
 }
 
 static PyObject *
 _proxy_subtype_new(PyTypeObject *type,
                    PyObject *obj,
-                   pg_getbufferfunc get_buffer)
+                   getbufferproc get_buffer)
 {
     PgBufproxyObject *self = (PgBufproxyObject *)type->tp_alloc(type, 0);
 
 
 static Py_buffer *
 _proxy_get_view(PgBufproxyObject *proxy) {
-    Pg_buffer *view_p = proxy->view_p;
+    Pg_buffer *pg_view_p = proxy->pg_view_p;
 
-    if (!view_p) {
-        view_p = PyMem_New(Pg_buffer, 1);
-        if (!view_p) {
+    if (!pg_view_p) {
+        pg_view_p = PyMem_New(Pg_buffer, 1);
+        if (!pg_view_p) {
             PyErr_NoMemory();
             return 0;
         }
-        view_p->consumer = (PyObject *)proxy;
-        if (proxy->get_buffer(proxy->obj, view_p, PyBUF_RECORDS_RO)) {
-            PyMem_Free(view_p);
+        pg_view_p->consumer = (PyObject *)proxy;
+        if (proxy->get_buffer(proxy->obj,
+                              (Py_buffer *)pg_view_p,
+                              PyBUF_RECORDS_RO)) {
+            PyMem_Free(pg_view_p);
             return 0;
         }
-        proxy->view_p = view_p;
+        proxy->pg_view_p = pg_view_p;
     }
     assert(((Py_buffer *)view_p)->len && ((Py_buffer *)view_p)->itemsize);
-    return (Py_buffer *)view_p;
+    return (Py_buffer *)pg_view_p;
 }
 
 static void
 _proxy_release_view(PgBufproxyObject *proxy) {
-    Pg_buffer *view_p = proxy->view_p;
+    Pg_buffer *pg_view_p = proxy->pg_view_p;
 
-    if (view_p) {
-        proxy->view_p = 0;
-        PgBuffer_Release(view_p);
-        PyMem_Free(view_p);
+    if (pg_view_p) {
+        proxy->pg_view_p = 0;
+        PgBuffer_Release(pg_view_p);
+        PyMem_Free(pg_view_p);
     }
 }
 
 static int
-_proxy_zombie_get_buffer(PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_proxy_zombie_get_buffer(PyObject *obj, Py_buffer *view_p, int flags)
 {
-    PyObject *proxy = pg_view_p->consumer;
+    PyObject *proxy = ((Pg_buffer *)view_p)->consumer;
 
-    ((Py_buffer *)pg_view_p)->obj = 0;
+    view_p->obj = 0;
     PyErr_Format (PyExc_RuntimeError,
                   "Attempted buffer export on <%s at %p, parent=<%s at %p>> "
                   "while deallocating it",
 proxy_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
     PyObject *obj = 0;
-    pg_getbufferfunc get_buffer = PgObject_GetBuffer;
+    getbufferproc get_buffer = (getbufferproc)PgObject_GetBuffer;
 
     if (!PyArg_ParseTuple(args, "O:Bufproxy", &obj)) {
         return 0;
     if (self->obj) {
         Py_VISIT(self->obj);
     }
-    if (self->view_p && ((Py_buffer *)self->view_p)->obj) /* conditional && */ {
-        Py_VISIT(((Py_buffer *)self->view_p)->obj);
+    if (self->pg_view_p && /* conditional && */
+        ((Py_buffer *)self->pg_view_p)->obj) {
+        Py_VISIT(((Py_buffer *)self->pg_view_p)->obj);
     }
     if (self->dict) {
         Py_VISIT(self->dict);
 static int
 proxy_getbuffer(PgBufproxyObject *self, Py_buffer *view_p, int flags)
 {
-    Pg_buffer *pg_obj_view_p = PyMem_New(Pg_buffer, 1);
-    Py_buffer *obj_view_p = (Py_buffer *)pg_obj_view_p;
+    Py_buffer *obj_view_p = PyMem_Malloc(sizeof (Pg_buffer));
 
     view_p->obj = 0;
-    if (!pg_obj_view_p) {
+    if (!obj_view_p) {
         PyErr_NoMemory();
         return -1;
     }
-    pg_obj_view_p->consumer = (PyObject *)self;
-    if (self->get_buffer(self->obj, pg_obj_view_p, flags)) {
-        PyMem_Free(pg_obj_view_p);
+    ((Pg_buffer *)obj_view_p)->consumer = (PyObject *)self;
+    if (self->get_buffer(self->obj, obj_view_p, flags)) {
+        PyMem_Free(obj_view_p);
         return -1;
     }
     Py_INCREF(self);
 static Py_ssize_t
 proxy_getreadbuf(PgBufproxyObject *self, Py_ssize_t _index, void **ptr)
 {
-    Py_buffer *view_p = (Py_buffer *)self->view_p;
+    Py_buffer *view_p = (Py_buffer *)self->pg_view_p;
     Py_ssize_t offset = 0;
     Py_ssize_t dim;
 
     if (seglen < 0) {
         return -1;
     }
-    if (seglen > 0 && ((Py_buffer *)self->view_p)->readonly) /* cond. && */ {
+    if (seglen > 0 &&  /* cond. && */
+        ((Py_buffer *)self->pg_view_p)->readonly) {
         PyErr_SetString(PyExc_ValueError, "buffer is not writeable");
         return -1;
     }
 /**** Public C api ***/
 
 static PyObject *
-PgBufproxy_New(PyObject *obj, pg_getbufferfunc get_buffer)
+PgBufproxy_New(PyObject *obj, getbufferproc get_buffer)
 {
     if (!get_buffer) {
         if (!obj) {
                             "required: both NULL instead");
             return 0;
         }
-        get_buffer = PgObject_GetBuffer;
+        get_buffer = (getbufferproc)PgObject_GetBuffer;
     }
     return _proxy_subtype_new(&PgBufproxy_Type, obj, get_buffer);
 }

File src/pgbufferproxy.h

  */
 #if !defined(PG_BUFPROXY_HEADER)
 
-typedef int (*pg_getbufferfunc)(PyObject *, struct pg_bufferinfo_s *, int);
-
 #define PYGAMEAPI_BUFPROXY_NUMSLOTS 4
 #define PYGAMEAPI_BUFPROXY_FIRSTSLOT 0
 
 #if !(defined(PYGAMEAPI_BUFPROXY_INTERNAL) || defined(NO_PYGAME_C_API))
 static void *PgBUFPROXY_C_API[PYGAMEAPI_BUFPROXY_NUMSLOTS];
 
-typedef PyObject *(*_pgbufproxy_new_t)(PyObject *, pg_getbufferfunc);
+typedef PyObject *(*_pgbufproxy_new_t)(PyObject *, getbufferproc);
 typedef PyObject *(*_pgbufproxy_get_obj_t)(PyObject *);
 typedef int (*_pgbufproxy_trip_t)(PyObject *);
 

File src/pygame.h

 #define PyBUF_READ  0x100
 #define PyBUF_WRITE 0x200
 #define PyBUF_SHADOW 0x400
+
+typedef int (*getbufferproc)(PyObject *, Py_buffer *, int);
+typedef void (*releasebufferproc)(Py_buffer *);
 #endif /* #if !defined(PyBUF_SIMPLE) */
 
+/* Flag indicating a Pg_buffer; used for assertions within callbacks */
+#ifndef NDEBUG
+#define PyBUF_PYGAME 0x4000
+#endif
+
 #define PyBUF_HAS_FLAG(f, F) (((f) & (F)) == (F))
 
-/* Array information exchange struct C type; inherits from Py_buffer */
-
-struct pg_bufferinfo_s;
-typedef void (*pg_releasebufferfunc)(struct bufferinfo *);
+/* Array information exchange struct C type; inherits from Py_buffer
+ *
+ * Pygame uses its own Py_buffer derived C struct as an internal representation
+ * of an imported array buffer. The extended Py_buffer allows for a
+ * per-instance release callback, 
+ */
+typedef void (*pybuffer_releaseproc)(Py_buffer *);
 
 typedef struct pg_bufferinfo_s {
     Py_buffer view;
     PyObject *consumer;                   /* Input: Borrowed reference */
-    pg_releasebufferfunc release_buffer;
+    pybuffer_releaseproc release_buffer;
 } Pg_buffer;
 
+/* Operating system specific adjustments
+ */
 // No signal()
 #if defined(__SYMBIAN32__) && defined(HAVE_SIGNAL_H)
 #undef HAVE_SIGNAL_H

File src/surface.c

 static PyObject *surf_get_pixels_address (PyObject *self,
                                           PyObject *closure);
 static int _view_kind (PyObject *obj, void *view_kind_vptr);
-static int _get_buffer_0D (PyObject *obj, Pg_buffer *pg_view_p, int flags);
-static int _get_buffer_1D (PyObject *obj, Pg_buffer *pg_view_p, int flags);
-static int _get_buffer_2D (PyObject *obj, Pg_buffer *pg_view_p, int flags);
-static int _get_buffer_3D (PyObject *obj, Pg_buffer *pg_view_p, int flags);
-static int _get_buffer_red (PyObject *obj, Pg_buffer *pg_view_p, int flags);
-static int _get_buffer_green (PyObject *obj, Pg_buffer *pg_view_p, int flags);
-static int _get_buffer_blue (PyObject *obj, Pg_buffer *pg_view_p, int flags);
-static int _get_buffer_alpha (PyObject *obj, Pg_buffer *pg_view_p, int flags);
+static int _get_buffer_0D (PyObject *obj, Py_buffer *view_p, int flags);
+static int _get_buffer_1D (PyObject *obj, Py_buffer *view_p, int flags);
+static int _get_buffer_2D (PyObject *obj, Py_buffer *view_p, int flags);
+static int _get_buffer_3D (PyObject *obj, Py_buffer *view_p, int flags);
+static int _get_buffer_red (PyObject *obj, Py_buffer *view_p, int flags);
+static int _get_buffer_green (PyObject *obj, Py_buffer *view_p, int flags);
+static int _get_buffer_blue (PyObject *obj, Py_buffer *view_p, int flags);
+static int _get_buffer_alpha (PyObject *obj, Py_buffer *view_p, int flags);
 static int _get_buffer_colorplane (PyObject *obj,
-                                   Pg_buffer *pg_view_p,
+                                   Py_buffer *view_p,
                                    int flags,
                                    char *name,
                                    Uint32 mask);
-static int _init_buffer(PyObject *surf, Pg_buffer *pg_view_p, int flags);
+static int _init_buffer(PyObject *surf, Py_buffer *view_p, int flags);
 static void _release_buffer(Py_buffer *view_p);
 static PyObject *_raise_get_view_ndim_error(int bitsize, SurfViewKind kind);
 
     SDL_PixelFormat *format;
     Uint32 mask = 0;
     SurfViewKind view_kind = VIEWKIND_2D;
-    pg_getbufferfunc get_buffer = 0;
+    getbufferproc get_buffer = 0;
 
     if (!PyArg_ParseTuple (args, "|O&", _view_kind, &view_kind)) {
         return 0;
 }
 
 static int
-_get_buffer_0D (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_0D (PyObject *obj, Py_buffer *view_p, int flags)
 {
     SDL_Surface *surface = PySurface_AsSurface (obj);
-    Py_buffer *view_p = (Py_buffer *)pg_view_p;
 
     view_p->obj = 0;
-    if (_init_buffer (obj, pg_view_p, flags)) {
+    if (_init_buffer (obj, view_p, flags)) {
         return -1;
     }
     view_p->buf = surface->pixels;
 }
 
 static int
-_get_buffer_1D (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_1D (PyObject *obj, Py_buffer *view_p, int flags)
 {
-    Py_buffer *view_p = (Py_buffer *)pg_view_p;
     SDL_Surface *surface = PySurface_AsSurface (obj);
     Py_ssize_t itemsize = surface->format->BytesPerPixel;
 
     view_p->obj = 0;
     if (itemsize == 1) {
-        return _get_buffer_0D (obj, pg_view_p, flags);
+        return _get_buffer_0D (obj, view_p, flags);
     }
-    if (_init_buffer (obj, pg_view_p, flags)) {
+    if (_init_buffer (obj, view_p, flags)) {
         return -1;
     }
     if (PyBUF_HAS_FLAG (flags, PyBUF_FORMAT)) {
 }
 
 static int
-_get_buffer_2D (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_2D (PyObject *obj, Py_buffer *view_p, int flags)
 {
-    Py_buffer *view_p = (Py_buffer *)pg_view_p;
     SDL_Surface *surface = PySurface_AsSurface (obj);
     int itemsize = surface->format->BytesPerPixel;
 
                              "A 2D surface view is not C contiguous");
             return -1;
         }
-        return _get_buffer_1D (obj, pg_view_p, flags);
+        return _get_buffer_1D (obj, view_p, flags);
     }
     if (!PyBUF_HAS_FLAG (flags, PyBUF_STRIDES)) {
             PyErr_SetString (PgExc_BufferError,
                          "This 2D surface view is not contiguous");
         return -1;
     }
-    if (_init_buffer (obj, pg_view_p, flags)) {
+    if (_init_buffer (obj, view_p, flags)) {
         return -1;
     }
     if (PyBUF_HAS_FLAG (flags, PyBUF_FORMAT)) {
 }
 
 static int
-_get_buffer_3D (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_3D (PyObject *obj, Py_buffer *view_p, int flags)
 {
     const int lilendian = (SDL_BYTEORDER == SDL_LIL_ENDIAN);
-    Py_buffer *view_p = (Py_buffer *)pg_view_p;
     SDL_Surface *surface = PySurface_AsSurface (obj);
     int pixelsize = surface->format->BytesPerPixel;
     char *startpixel = (char *)surface->pixels;
                          "A 3D surface view is not contiguous");
         return -1;
     }
-    if (_init_buffer (obj, pg_view_p, flags)) {
+    if (_init_buffer (obj, view_p, flags)) {
         return -1;
     }
     if (PyBUF_HAS_FLAG (flags, PyBUF_FORMAT)) {
 }
 
 static int
-_get_buffer_red (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_red (PyObject *obj, Py_buffer *view_p, int flags)
 {
     return _get_buffer_colorplane(obj,
-                                  pg_view_p,
+                                  view_p,
                                   flags,
                                   "red",
                                   PySurface_AsSurface (obj)->format->Rmask);
 }
 
 static int
-_get_buffer_green (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_green (PyObject *obj, Py_buffer *view_p, int flags)
 {
     return _get_buffer_colorplane(obj,
-                                  pg_view_p,
+                                  view_p,
                                   flags,
                                   "green",
                                   PySurface_AsSurface (obj)->format->Gmask);
 }
 
 static int
-_get_buffer_blue (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_blue (PyObject *obj, Py_buffer *view_p, int flags)
 {
     return _get_buffer_colorplane(obj,
-                                  pg_view_p,
+                                  view_p,
                                   flags,
                                   "blue",
                                   PySurface_AsSurface (obj)->format->Bmask);
 }
 
 static int
-_get_buffer_alpha (PyObject *obj, Pg_buffer *pg_view_p, int flags)
+_get_buffer_alpha (PyObject *obj, Py_buffer *view_p, int flags)
 {
     return _get_buffer_colorplane(obj,
-                                  pg_view_p,
+                                  view_p,
                                   flags,
                                   "alpha",
                                   PySurface_AsSurface (obj)->format->Amask);
 
 static int
 _get_buffer_colorplane (PyObject *obj,
-                        Pg_buffer *pg_view_p,
+                        Py_buffer *view_p,
                         int flags,
                         char *name,
                         Uint32 mask)
 {
     const int lilendian = (SDL_BYTEORDER == SDL_LIL_ENDIAN);
-    Py_buffer *view_p = (Py_buffer *)pg_view_p;
     SDL_Surface *surface = PySurface_AsSurface (obj);
     int pixelsize = surface->format->BytesPerPixel;
     char *startpixel = (char *)surface->pixels;
         return -1;
 #endif
     }
-    if (_init_buffer (obj, pg_view_p, flags)) {
+    if (_init_buffer (obj, view_p, flags)) {
         return -1;
     }
     view_p->buf = startpixel;
 }
 
 static int
-_init_buffer (PyObject *surf, Pg_buffer *pg_view_p, int flags)
+_init_buffer (PyObject *surf, Py_buffer *view_p, int flags)
 {
     PyObject *consumer;
     Pg_bufferinternal *internal;
 
-    assert (surf &&
-            pg_view_p &&
-            PyObject_IsInstance (surf, (PyObject *)&PySurface_Type));
-    consumer = pg_view_p->consumer;
+    assert (surf);
+    assert (pg_view_p);
+    assert (PyObject_IsInstance (surf, (PyObject *)&PySurface_Type));
+    assert (PyBUF_HAS_FLAG (flags, PyBUF_PYGAME));
+    consumer = ((Pg_buffer *)view_p)->consumer;
     assert (consumer);
     internal = PyMem_New (Pg_bufferinternal, 1);
     if (!internal) {
         return -1;
     }
     if (PyBUF_HAS_FLAG (flags, PyBUF_ND)) {
-        ((Py_buffer *)pg_view_p)->shape = internal->mem;
+        view_p->shape = internal->mem;
         if (PyBUF_HAS_FLAG (flags, PyBUF_STRIDES)) {
-            ((Py_buffer *)pg_view_p)->strides = internal->mem + 3;
+            view_p->strides = internal->mem + 3;
         }
         else {
-            ((Py_buffer *)pg_view_p)->strides = 0;
+            view_p->strides = 0;
         }
     }
     else {
-        ((Py_buffer *)pg_view_p)->shape = 0;
-        ((Py_buffer *)pg_view_p)->strides = 0;
+        view_p->shape = 0;
+        view_p->strides = 0;
     }
-    ((Py_buffer *)pg_view_p)->ndim = 0;
-    ((Py_buffer *)pg_view_p)->format = 0;
-    ((Py_buffer *)pg_view_p)->suboffsets = 0;
-    ((Py_buffer *)pg_view_p)->internal = internal;
-    pg_view_p->release_buffer = _release_buffer;
+    view_p->ndim = 0;
+    view_p->format = 0;
+    view_p->suboffsets = 0;
+    view_p->internal = internal;
+    ((Pg_buffer *)view_p)->release_buffer = _release_buffer;
     return 0;
 }