<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Mar 10, 2015 at 11:34 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 | 119 ++++++++++++++++++++++++++++++++++++++-------------- src/mesa/main/pbo.h | 14 +++++++ 2 files changed, 101 insertions(+), 32 deletions(-) diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c index 259f763..46121a2 100644 <div><div class="h5">--- 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. </div></div></blockquote><div>Maybe say something like "If there are errors, return false, else return true"? This is important because in other parts of the driver (like main/teximage.c), error checking functions return GL_TRUE if an error is found and GL_FALSE if one isn't found. (Confusing, I know :< ) </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> * \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); </div></div>@@ -188,24 +183,85 @@ _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); - } else { - _mesa_error(ctx, GL_INVALID_OPERATION, </blockquote><div>You changed the order of the conditions from the original version. In the original, if (_mesa_is_bufferobj), then say "out of bounds PBO access." If (!_mesa_is_bufferobj), say "out of bounds access: bufSize is too small." You have them switched around here. </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); + } else { + _mesa_error(ctx, GL_INVALID_OPERATION, + "%s(out of bounds access)", + where); } - 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); - return NULL; + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)", + where); + 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. </blockquote><div>Same here as above. Say something about the return value (true or false). </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + */ +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(invalid PBO access)", + where); + return false; + } + + 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); <div class="HOEnZb"><div class="h5">+ 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; } </div></div> ptr = _mesa_map_pbo_source(ctx, unpack, ptr); <div class="HOEnZb"><div class="h5">@@ -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 </div></div></blockquote></div> </div></div>