<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Mar 5, 2015 at 12:20 AM, Eduardo Lima Mitev <<a href="mailto:elima@igalia.com" target="_blank">elima@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Internal PBO functions such as _mesa_map_validate_pbo_source() and _mesa_validate_pbo_compressed_teximage() perform validation and buffer mapping within the same call. This patch takes out the validation into separate functions to allow reuse of functionality by other code (i.e, gl(Compressed)Tex(Sub)Image). --- src/mesa/main/pbo.c | 117 ++++++++++++++++++++++++++++++++++++++-------------- src/mesa/main/pbo.h | 14 +++++++ 2 files changed, 100 insertions(+), 31 deletions(-) diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c index 259f763..5ae248c 100644 --- a/src/mesa/main/pbo.c +++ b/src/mesa/main/pbo.c @@ -164,23 +164,18 @@ _mesa_map_pbo_source(struct gl_context *ctx, return buf; } - /** - * Combine PBO-read validation and mapping. + * Perform PBO validation for read operations with uncompressed textures. * If any GL errors are detected, they'll be recorded and NULL returned. * \sa _mesa_validate_pbo_access - * \sa _mesa_map_pbo_source - * A call to this function should have a matching call to - * _mesa_unmap_pbo_source(). */ -const GLvoid * -_mesa_map_validate_pbo_source(struct gl_context *ctx, - GLuint dimensions, - const struct gl_pixelstore_attrib *unpack, - GLsizei width, GLsizei height, GLsizei depth, - GLenum format, GLenum type, - GLsizei clientMemSize, - const GLvoid *ptr, const char *where) +bool +_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions, + const struct gl_pixelstore_attrib *unpack, + GLsizei width, GLsizei height, GLsizei depth, + GLenum format, GLenum type, + GLsizei clientMemSize, + const GLvoid *ptr, const char *where) { assert(dimensions == 1 || dimensions == 2 || dimensions == 3); @@ -188,26 +183,87 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx, format, type, clientMemSize, ptr)) { if (_mesa_is_bufferobj(unpack->BufferObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(out of bounds PBO access)", where); </blockquote><div>Changing the error to "%s%uD, where, dimensions" is not ideal because the other function that calls _mesa_map_validate_pbo_source is _mesa_PolygonStipple, and printing "glPolygonStipple2D" doesn't make sense because the name of the function is "glPolygonStipple." </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + "%s%uD(out of bounds PBO access)", + where, dimensions); } else { _mesa_error(ctx, GL_INVALID_OPERATION, </blockquote><div>Why did you remove "bufSize (%d) is too small"? If we are going to remove that, maybe we should remove the if, then, else block because the error messages are pretty much the same. I recommend keeping the original error messages for both and moving the "%sD" up to the calling functions (for example, texture_error_check). </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> - "%s(out of bounds access: bufSize (%d) is too small)", - where, clientMemSize); + "%s%uD(out of bounds access)", + where, dimensions); } - return NULL; + return false; } if (!_mesa_is_bufferobj(unpack->BufferObj)) { /* non-PBO access: no further validation to be done */ - return ptr; + return true; } if (_mesa_check_disallowed_mapping(unpack->BufferObj)) { /* buffer is already mapped - that's an error */ - _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)", where); + _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(PBO is mapped)", + where, dimensions); + return false; + } + + return true; +} + +/** + * Perform PBO validation for read operations with compressed textures. + * If any GL errors are detected, they'll be recorded and NULL returned. + */ +bool +_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint dimensions, + const struct gl_pixelstore_attrib *unpack, + GLsizei imageSize, const GLvoid *pixels, + const char *where) +{ + if (!_mesa_is_bufferobj(unpack->BufferObj)) { + /* not using a PBO */ + return true; + } + + if ((const GLubyte *) pixels + imageSize > + ((const GLubyte *) 0) + unpack->BufferObj->Size) { + /* out of bounds read! */ + _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(invalid PBO access)", + where, dimensions); </blockquote><div>This should be return false. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> return NULL; } + if (_mesa_check_disallowed_mapping(unpack->BufferObj)) { + /* buffer is already mapped - that's an error */ + _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(PBO is mapped)", + where, dimensions); + return false; + } + + return true; +} + +/** + * Perform PBO-read mapping. + * If any GL errors are detected, they'll be recorded and NULL returned. + * \sa _mesa_validate_pbo_source + * \sa _mesa_map_pbo_source + * A call to this function should have a matching call to + * _mesa_unmap_pbo_source(). + */ +const GLvoid * +_mesa_map_validate_pbo_source(struct gl_context *ctx, + GLuint dimensions, + const struct gl_pixelstore_attrib *unpack, + GLsizei width, GLsizei height, GLsizei depth, + GLenum format, GLenum type, + GLsizei clientMemSize, + const GLvoid *ptr, const char *where) +{ + if (!_mesa_validate_pbo_source(ctx, dimensions, unpack, + width, height, depth, format, type, + clientMemSize, ptr, where)) { + return NULL; + } + ptr = _mesa_map_pbo_source(ctx, unpack, ptr); return ptr; } @@ -381,28 +437,27 @@ _mesa_validate_pbo_compressed_teximage(struct gl_context *ctx, { GLubyte *buf; + if (!_mesa_validate_pbo_source_compressed(ctx, dimensions, packing, + imageSize, pixels, funcName)) { + /* error is already set during validation */ + return NULL; + } + if (!_mesa_is_bufferobj(packing->BufferObj)) { /* not using a PBO - return pointer unchanged */ return pixels; } - if ((const GLubyte *) pixels + imageSize > - ((const GLubyte *) 0) + packing->BufferObj->Size) { - /* out of bounds read! */ - _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(invalid PBO access)", - funcName, dimensions); - return NULL; - } buf = (GLubyte*) ctx->Driver.MapBufferRange(ctx, 0, packing->BufferObj->Size, GL_MAP_READ_BIT, packing->BufferObj, MAP_INTERNAL); - if (!buf) { - _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(PBO is mapped)", funcName, - dimensions); - return NULL; - } + + /* Validation above already checked that PBO is not mapped, so buffer + * should not be null. + */ + assert(buf); return ADD_POINTERS(buf, pixels); } diff --git a/src/mesa/main/pbo.h b/src/mesa/main/pbo.h index 9851ef1..b3f24e6 100644 --- a/src/mesa/main/pbo.h +++ b/src/mesa/main/pbo.h @@ -92,4 +92,18 @@ _mesa_unmap_teximage_pbo(struct gl_context *ctx, const struct gl_pixelstore_attrib *unpack); +extern bool +_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions, + const struct gl_pixelstore_attrib *unpack, + GLsizei width, GLsizei height, GLsizei depth, + GLenum format, GLenum type, + GLsizei clientMemSize, + const GLvoid *ptr, const char *where); + +extern bool +_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint dimensions, + const struct gl_pixelstore_attrib *unpack, + GLsizei imageSize, const GLvoid *ptr, + const char *where); + #endif -- 2.1.3 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>