[Mesa-dev] [PATCH v2 2/3] mesa: validate size parameters for glTexStorage*Multisample

[Tapani Pälli]

On 08/13/2015 11:54 AM, Timothy Arceri wrote:
> I've sent a couple of follow-up patches I notice when reviewing this.
>
>
> On Thu, 2015-08-13 at 09:30 +0300, Tapani Pälli wrote:
>> v2: code cleanup
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/teximage.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index d35dc12..add7438 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -5782,6 +5782,18 @@ _mesa_TexImage3DMultisample(GLenum target, GLsizei
>> samples,
>>                                      "glTexImage3DMultisample");
>>   }
>>
>> +static bool
>> +valid_texstorage_ms_parameters(GLsizei width, GLsizei height, GLsizei
>> depth,
>> +                               GLsizei samples, unsigned dims)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +
>> +   if (_mesa_tex_storage_invalid_dim(width, height, depth) || samples < 1)
>
>
> Rather than do the samples < 1 check here you should move it to
>   _mesa_texture_image_multisample as the spec says "An INVALID_VALUE error is
> generated ifsamplesis zero." for glTexImage*Multisample too.

_mesa_texture_image_multisample uses _mesa_check_sample_count() which 
errors on negative values like some of the functions are specified, I'll 
need to go through the funcs again to see which one it was but there was 
inconsistency, some require < 0 and some already error on 0.


> With that change you could change all the calls below to something like
>
>     if (_mesa_tex_storage_invalid_dim(width, height, depth)) {
>        _mesa_error(ctx, GL_INVALID_VALUE, "glTexStorage2DMultisample()"
>        return;
>     }
>
>> {
>> +      _mesa_error(ctx, GL_INVALID_VALUE, "glTexStorage%uDMultisample()",
>> dims);
>> +      return false;
>> +   }
>> +   return true;
>> +}
>>   void GLAPIENTRY
>>   _mesa_TexStorage2DMultisample(GLenum target, GLsizei samples,
>> @@ -5795,6 +5807,9 @@ _mesa_TexStorage2DMultisample(GLenum target, GLsizei
>> samples,
>>      if (!texObj)
>>         return;
>>
>> +   if (!valid_texstorage_ms_parameters(width, height, 1, samples, 2))
>> +      return;
>> +
>>      _mesa_texture_image_multisample(ctx, 2, texObj, target, samples,
>>                                      internalformat, width, height, 1,
>>                                      fixedsamplelocations, GL_TRUE,
>> @@ -5814,6 +5829,9 @@ _mesa_TexStorage3DMultisample(GLenum target, GLsizei
>> samples,
>>      if (!texObj)
>>         return;
>>
>> +   if (!valid_texstorage_ms_parameters(width, height, depth, samples, 3))
>> +      return;
>> +
>>      _mesa_texture_image_multisample(ctx, 3, texObj, target, samples,
>>                                      internalformat, width, height, depth,
>>                                      fixedsamplelocations, GL_TRUE,
>> @@ -5834,6 +5852,9 @@ _mesa_TextureStorage2DMultisample(GLuint texture,
>> GLsizei samples,
>>      if (!texObj)
>>         return;
>>
>> +   if (!valid_texstorage_ms_parameters(width, height, 1, samples, 2))
>> +      return;
>> +
>>      _mesa_texture_image_multisample(ctx, 2, texObj, texObj->Target, samples,
>>                                      internalformat, width, height, 1,
>>                                      fixedsamplelocations, GL_TRUE,
>> @@ -5855,6 +5876,9 @@ _mesa_TextureStorage3DMultisample(GLuint texture,
>> GLsizei samples,
>>      if (!texObj)
>>         return;
>>
>> +   if (!valid_texstorage_ms_parameters(width, height, depth, samples, 3))
>> +      return;
>> +
>>      _mesa_texture_image_multisample(ctx, 3, texObj, texObj->Target, samples,
>>                                      internalformat, width, height, depth,
>>                                      fixedsamplelocations, GL_TRUE,