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

[Tapani Pälli]

On 08/13/2015 12:14 PM, Tapani Pälli wrote:
>
>
> 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.

It was RenderbufferStorageMultisample that should error on < 0, I guess 
nothing else (?) It could use it's own check and rest could use generic 
sample check. Would need to see also what possible Piglit tests for 
these are expecting and correct those if they are according to old specs.


>
>> 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,
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev