[Mesa-dev] [PATCH 4/4 v2] mesa, intel: use _mesa_image_offset() for PBOs

[nobled]

This avoids forming invalid pointers needlessly, which even if
never dereferenced is undefined behavior. It also makes
_mesa_validate_pbo_access() more comprehensible.
---
v2: Now rebased on top of the recent commit 9024d8af0ae.

 src/mesa/drivers/dri/intel/intel_pixel_read.c |    5 ++-
 src/mesa/main/pbo.c                           |   27 +++++++++++++-----------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_pixel_read.c
b/src/mesa/drivers/dri/intel/intel_pixel_read.c
index 803d714..1107017 100644
--- a/src/mesa/drivers/dri/intel/intel_pixel_read.c
+++ b/src/mesa/drivers/dri/intel/intel_pixel_read.c
@@ -120,8 +120,9 @@ do_blit_readpixels(struct gl_context * ctx,
 	 rowLength = -rowLength;
    }

-   dst_offset = (GLintptr) _mesa_image_address(2, pack, pixels, width, height,
-					       format, type, 0, 0, 0);
+   dst_offset = (GLintptr)pixels;
+   dst_offset += _mesa_image_offset(2, pack, width, height,
+				    format, type, 0, 0, 0);

    if (!_mesa_clip_copytexsubimage(ctx,
 				   &dst_x, &dst_y,
diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
index 41ff2ff..28e1a2c 100644
--- a/src/mesa/main/pbo.c
+++ b/src/mesa/main/pbo.c
@@ -68,8 +68,8 @@ _mesa_validate_pbo_access(GLuint dimensions,
                           GLenum format, GLenum type, GLsizei clientMemSize,
                           const GLvoid *ptr)
 {
-   const GLvoid *start, *end, *offset;
-   const GLubyte *sizeAddr; /* buffer size, cast to a pointer */
+   /* unsigned, to detect overflow/wrap-around */
+   uintptr_t start, end, offset, size;

    /* If no PBO is bound, 'ptr' is a pointer to client memory containing
       'clientMemSize' bytes.
@@ -78,10 +78,10 @@ _mesa_validate_pbo_access(GLuint dimensions,
     */
    if (!_mesa_is_bufferobj(pack->BufferObj)) {
       offset = 0;
-      sizeAddr = ((const GLubyte *) 0) + clientMemSize;
+      size = clientMemSize;
    } else {
       offset = ptr;
-      sizeAddr = ((const GLubyte *) 0) + pack->BufferObj->Size;
+      size = pack->BufferObj->Size;
       /* The ARB_pixel_buffer_object spec says:
        *    "INVALID_OPERATION is generated by ColorTable, ColorSubTable,
        *    ConvolutionFilter2D, ConvolutionFilter1D, SeparableFilter2D,
@@ -93,27 +93,30 @@ _mesa_validate_pbo_access(GLuint dimensions,
        *    parameter."
        */
       if (type != GL_BITMAP &&
-          ((GLintptr)offset % _mesa_sizeof_packed_type(type)))
+          (offset % _mesa_sizeof_packed_type(type)))
          return GL_FALSE;
    }

-   if (sizeAddr == 0)
+   if (size == 0)
       /* no buffer! */
       return GL_FALSE;

    /* get the offset to the first pixel we'll read/write */
-   start = _mesa_image_address(dimensions, pack, offset, width, height,
-                               format, type, 0, 0, 0);
+   start = _mesa_image_offset(dimensions, pack, width, height,
+                              format, type, 0, 0, 0);

    /* get the offset to just past the last pixel we'll read/write */
-   end =  _mesa_image_address(dimensions, pack, offset, width, height,
-                              format, type, depth-1, height-1, width);
+   end =  _mesa_image_offset(dimensions, pack, width, height,
+                             format, type, depth-1, height-1, width);

-   if ((const GLubyte *) start > sizeAddr) {
+   start += offset;
+   end += offset;
+
+   if (start > size) {
       /* This will catch negative values / wrap-around */
       return GL_FALSE;
    }
-   if ((const GLubyte *) end > sizeAddr) {
+   if (end > size) {
       /* Image read/write goes beyond end of buffer */
       return GL_FALSE;
    }
-- 
1.7.6.msysgit.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-mesa-intel-use-_mesa_image_offset-for-PBOs.patch
Type: application/octet-stream
Size: 3827 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111019/a6541746/attachment.obj>