<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 12, 2015 at 12:14 AM, Eduardo Lima Mitev <span dir="ltr"><<a href="mailto:elima@igalia.com" target="_blank">elima@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Internal PBO functions such as _mesa_map_validate_pbo_source() and<br>
_mesa_validate_pbo_compressed_teximage() perform validation and buffer mapping<br>
within the same call.<br>
<br>
This patch takes out the validation into separate functions to allow reuse<br>
of functionality by other code (i.e, gl(Compressed)Tex(Sub)Image).<br>
---<br>
</span> src/mesa/main/pbo.c | 117 ++++++++++++++++++++++++++++++++++++++--------------<br>
 src/mesa/main/pbo.h |  14 +++++++<br>
 2 files changed, 100 insertions(+), 31 deletions(-)<br>
<br>
diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c<br>
index 259f763..85c32a1 100644<br>
<span class="">--- a/src/mesa/main/pbo.c<br>
+++ b/src/mesa/main/pbo.c<br>
@@ -164,23 +164,18 @@ _mesa_map_pbo_source(struct gl_context *ctx,<br>
    return buf;<br>
 }<br>
<br>
-<br>
 /**<br>
- * Combine PBO-read validation and mapping.<br>
</span><span class="">- * If any GL errors are detected, they'll be recorded and NULL returned.<br>
</span><span class="">+ * Perform PBO validation for read operations with uncompressed textures.<br>
</span>+ * If any GL errors are detected, false is returned, otherwise returns true.<br>
<div><div class="h5">  * \sa _mesa_validate_pbo_access<br>
- * \sa _mesa_map_pbo_source<br>
- * A call to this function should have a matching call to<br>
- * _mesa_unmap_pbo_source().<br>
  */<br>
-const GLvoid *<br>
-_mesa_map_validate_pbo_source(struct gl_context *ctx,<br>
-                              GLuint dimensions,<br>
-                              const struct gl_pixelstore_attrib *unpack,<br>
-                              GLsizei width, GLsizei height, GLsizei depth,<br>
-                              GLenum format, GLenum type,<br>
-                              GLsizei clientMemSize,<br>
-                              const GLvoid *ptr, const char *where)<br>
+bool<br>
+_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions,<br>
+                          const struct gl_pixelstore_attrib *unpack,<br>
+                          GLsizei width, GLsizei height, GLsizei depth,<br>
+                          GLenum format, GLenum type,<br>
+                          GLsizei clientMemSize,<br>
+                          const GLvoid *ptr, const char *where)<br>
 {<br>
    assert(dimensions == 1 || dimensions == 2 || dimensions == 3);<br>
<br>
@@ -188,24 +183,85 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx,<br>
                                   format, type, clientMemSize, ptr)) {<br>
       if (_mesa_is_bufferobj(unpack->BufferObj)) {<br>
          _mesa_error(ctx, GL_INVALID_OPERATION,<br></div></div></blockquote><div>Add the "PBO" back to this comment. <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
-                     "%s(out of bounds PBO access)", where);<br>
</div></div><span class="">+                     "%s(out of bounds access)",<br>
+                     where);<br>
</span>       } else {<br>
          _mesa_error(ctx, GL_INVALID_OPERATION,<br>
<span class="">                      "%s(out of bounds access: bufSize (%d) is too small)",<br>
                      where, clientMemSize);<br>
       }<br>
</span><span class="">-      return NULL;<br>
+      return false;<br>
    }<br>
<br>
    if (!_mesa_is_bufferobj(unpack->BufferObj)) {<br>
       /* non-PBO access: no further validation to be done */<br>
-      return ptr;<br>
+      return true;<br>
    }<br>
<br>
    if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {<br>
       /* buffer is already mapped - that's an error */<br>
-      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)", where);<br>
-      return NULL;<br>
+      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",<br>
+                  where);<br>
+      return false;<br>
+   }<br>
+<br>
+   return true;<br>
+}<br>
+<br>
+/**<br>
+ * Perform PBO validation for read operations with compressed textures.<br>
</span>+ * If any GL errors are detected, false is returned, otherwise returns true.<br>
<div class="HOEnZb"><div class="h5">+ */<br>
+bool<br>
+_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint dimensions,<br>
+                                     const struct gl_pixelstore_attrib *unpack,<br>
+                                     GLsizei imageSize, const GLvoid *pixels,<br>
+                                     const char *where)<br>
+{<br>
+   if (!_mesa_is_bufferobj(unpack->BufferObj)) {<br>
+      /* not using a PBO */<br>
+      return true;<br>
+   }<br>
+<br>
+   if ((const GLubyte *) pixels + imageSize ><br>
+       ((const GLubyte *) 0) + unpack->BufferObj->Size) {<br>
+      /* out of bounds read! */<br>
+      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid PBO access)",<br>
+                  where);<br>
+      return false;<br>
+   }<br>
+<br>
+   if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {<br>
+      /* buffer is already mapped - that's an error */<br>
+      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",<br>
+                  where);<br>
+      return false;<br>
+   }<br>
+<br>
+   return true;<br>
+}<br>
+<br>
+/**<br>
+ * Perform PBO-read mapping.<br>
+ * If any GL errors are detected, they'll be recorded and NULL returned.<br>
+ * \sa _mesa_validate_pbo_source<br>
+ * \sa _mesa_map_pbo_source<br>
+ * A call to this function should have a matching call to<br>
+ * _mesa_unmap_pbo_source().<br>
+ */<br>
+const GLvoid *<br>
+_mesa_map_validate_pbo_source(struct gl_context *ctx,<br>
+                              GLuint dimensions,<br>
+                              const struct gl_pixelstore_attrib *unpack,<br>
+                              GLsizei width, GLsizei height, GLsizei depth,<br>
+                              GLenum format, GLenum type,<br>
+                              GLsizei clientMemSize,<br>
+                              const GLvoid *ptr, const char *where)<br>
+{<br>
+   if (!_mesa_validate_pbo_source(ctx, dimensions, unpack,<br>
+                                  width, height, depth, format, type,<br>
+                                  clientMemSize, ptr, where)) {<br>
+     return NULL;<br>
    }<br>
<br>
    ptr = _mesa_map_pbo_source(ctx, unpack, ptr);<br>
@@ -381,28 +437,27 @@ _mesa_validate_pbo_compressed_teximage(struct gl_context *ctx,<br>
 {<br>
    GLubyte *buf;<br>
<br>
+   if (!_mesa_validate_pbo_source_compressed(ctx, dimensions, packing,<br>
+                                             imageSize, pixels, funcName)) {<br>
+     /* error is already set during validation */<br>
+      return NULL;<br>
+   }<br>
+<br>
    if (!_mesa_is_bufferobj(packing->BufferObj)) {<br>
       /* not using a PBO - return pointer unchanged */<br>
       return pixels;<br>
    }<br>
-   if ((const GLubyte *) pixels + imageSize ><br>
-       ((const GLubyte *) 0) + packing->BufferObj->Size) {<br>
-      /* out of bounds read! */<br>
-      _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(invalid PBO access)",<br>
-                  funcName, dimensions);<br>
-      return NULL;<br>
-   }<br>
<br>
    buf = (GLubyte*) ctx->Driver.MapBufferRange(ctx, 0,<br>
                                               packing->BufferObj->Size,<br>
                                               GL_MAP_READ_BIT,<br>
                                               packing->BufferObj,<br>
                                                MAP_INTERNAL);<br>
-   if (!buf) {<br>
-      _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(PBO is mapped)", funcName,<br>
-                  dimensions);<br>
-      return NULL;<br>
-   }<br>
+<br>
+   /* Validation above already checked that PBO is not mapped, so buffer<br>
+    * should not be null.<br>
+    */<br>
+   assert(buf);<br>
<br>
    return ADD_POINTERS(buf, pixels);<br>
 }<br>
diff --git a/src/mesa/main/pbo.h b/src/mesa/main/pbo.h<br>
index 9851ef1..b3f24e6 100644<br>
--- a/src/mesa/main/pbo.h<br>
+++ b/src/mesa/main/pbo.h<br>
@@ -92,4 +92,18 @@ _mesa_unmap_teximage_pbo(struct gl_context *ctx,<br>
                          const struct gl_pixelstore_attrib *unpack);<br>
<br>
<br>
+extern bool<br>
+_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions,<br>
+                          const struct gl_pixelstore_attrib *unpack,<br>
+                          GLsizei width, GLsizei height, GLsizei depth,<br>
+                          GLenum format, GLenum type,<br>
+                          GLsizei clientMemSize,<br>
+                          const GLvoid *ptr, const char *where);<br>
+<br>
+extern bool<br>
+_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint dimensions,<br>
+                                     const struct gl_pixelstore_attrib *unpack,<br>
+                                     GLsizei imageSize, const GLvoid *ptr,<br>
+                                     const char *where);<br>
+<br>
 #endif<br>
--<br>
2.1.3<br>
<br>
</div></div></blockquote></div>With that fixed,<br><br></div><div class="gmail_extra">Reviewed-by: Laura Ekstrand <<a href="mailto:laura@jlekstrand.net">laura@jlekstrand.net</a>><br></div></div>