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

[Ian Romanick]

On 08/13/2015 02:28 AM, Tapani Pälli wrote:
> 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.

That's probably the best way.  RenderbufferStorageMultisample is a
little weird by having samples=0 mean non-multisampled.