[Mesa-dev] [PATCH] mesa: generate error if pbo offset is not aligned with the size of specified type

[Ian Romanick]

On 10/13/2011 06:46 PM, Yuanhan Liu wrote:
> Signed-off-by: Yuanhan Liu<yuanhan.liu at linux.intel.com>
> ---
>   src/mesa/main/pbo.c |    5 +++++
>   1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
> index 4e7e6f9..4611c5d 100644
> --- a/src/mesa/main/pbo.c
> +++ b/src/mesa/main/pbo.c
> @@ -82,6 +82,11 @@ _mesa_validate_pbo_access(GLuint dimensions,
>      } else {
>         offset = ptr;
>         sizeAddr = ((const GLubyte *) 0) + pack->BufferObj->Size;

In cases where an error is generated for non-obvious reasons, it is 
helpful to quote a bit of spec language.  In this case, the language in 
the GL spec is spread over a number of sections, but the language in the 
ARB_pixel_buffer_object spec is all in one place.

       /* The ARB_pixel_buffer_object spec says:
        *
        *    "INVALID_OPERATION is generated by ColorTable, ColorSubTable,
        *    ConvolutionFilter2D, ConvolutionFilter1D, SeparableFilter2D,
        *    TexImage1D, TexImage2D, TexImage3D, TexSubImage1D,
        *    TexSubImage2D, TexSubImage3D, and DrawPixels if the current
        *    PIXEL_UNPACK_BUFFER_BINDING_ARB value is non-zero and the data
        *    parameter is not evenly divisible into the number of basic 
machine
        *    units needed to store in memory a datum indicated by the type
        *    parameter."
        */

> +      if (_mesa_sizeof_type(type)&&
> +          ((GLuint)offset % _mesa_sizeof_type(type))) {

Two things:

1. Rather than call _mesa_sizeof_type twice, call it once and save the 
result.

2. It's not obvious why _mesa_sizeof_type needs to be tested against 0. 
  Is this just for the type == GL_BITMAP case?  If so, the test should be:

       if (type != GL_BITMAP &&
           ((GLuint)offset % _mesa_sizeof_type(type) != 0) {

That just makes things more explicit for the next person that reads this 
code.

> +         /* offset not aligned with type size */
> +         return GL_FALSE;
> +      }
>      }
>
>      if (sizeAddr == 0)