On 03/07/2014 09:13 PM, Eric Anholt wrote:
Tapani Pälli <tapani.pa...@intel.com> writes:

I think the way I'd write this comment is "Mapping from GL uniform
locations returned by glUniformLocation to UniformStorage entries.
Arrays will have multiple contiguous slots in the UniformRemapTable, all
pointing to the same UniformStorage entry."

("Remap" is vague, and I ended up reading the code to figure out what
this was mapping from.)

Thanks, I'll change the comment and the other comment error you found.

diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index 8cc5da7..700a333 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -246,6 +246,13 @@ validate_uniform_parameters(struct gl_context *ctx,
        return false;
     }
+ /* Check that the given location is in bounds of uniform remap table. */
+   if (location >= (GLint) shProg->NumUniformRemapTable) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
+                 caller, location);
+      return false;
+   }
+
     _mesa_uniform_split_location_offset(shProg, location, loc, array_index);
if (*loc >= shProg->NumUserUniformStorage) {
Couldn't the new test just replace the *loc >=
shProg->NumUserUniformStorage test?  We're always going to have a loc
pointing to a valid value, since you looked loc up from the remaptable.

True, I will remove the old check for *loc as that one will be always correct. New check has to be before split function call as otherwise assert introduced there will trigger when user given location is off bounds.

// Tapani

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to