[Mesa-stable] [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon

[Tapani Pälli]

On 08/08/2017 05:20 PM, Emil Velikov wrote:
> On 8 August 2017 at 08:21, Tapani Pälli <tapani.palli at intel.com> wrote:
>>
>> On 08/05/2017 02:25 AM, Emil Velikov wrote:
>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>
>>> As mentioned in previous commit the negative tests in dEQP expect the
>>> arguments to be evaluated in particular order.
>>>
>>> Namely - first the dpy, then the config, followed by the surface/window.
>>>
>>> Move the check further down or executing the test below will produce
>>> the following error.
>>>
>>>      dEQP-EGL.functional.negative_api.create_pbuffer_surface
>>>
>>>
>>>      <Section Name="Test2" Description="EGL_BAD_CONFIG is generated if
>>> config is not an EGL frame buffer configuration">
>>>         <Text>eglCreateWindowSurface(0x9bfff0f150, 0xffffffffffffffff,
>>> 0x0000000000000000, { EGL_NONE });</Text>
>>>         <Text>// 0x0000000000000000 returned</Text>
>>>         <Text>// ERROR expected: EGL_BAD_CONFIG, Got:
>>> EGL_BAD_NATIVE_WINDOW</Text>
>>>      </Section>
>>>
>>> Cc: <mesa-stable at lists.freedesktop.org>
>>> Cc: Mark Janes <mark.a.janes at intel.com>
>>> Cc: Chad Versace <chadversary at chromium.org>
>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>> ---
>>> Mark,
>>>
>>> IMHO the CI does the impossible and passes the test. Perhaps it's worth
>>> looking into how/why it does so - I don't know.
>>
>> For me the above test is passing fine on x11 (x11_egl).
>>
> Barring local changes i cannot see how that's possible.
>
> The test itself is as below, while Mesa does the window (NULL) check first.
>
> expectNoSurface(eglCreateWindowSurface(display, (EGLConfig)-1,
> DE_NULL, s_emptyAttribList));
> expectError(EGL_BAD_CONFIG);

My deqp has no such line, I'm pulling from:
https://android.googlesource.com/platform/external/deqp
and using the master branch.

I think you might be missing commit 
15a19da0d33b9f2abfd8eb9ea21f2397db5cadcb that says "Remove tests cases 
and checks assuming native handles to be invalid."?


> Then again, I've spotted a few extra bits that we're not handling
> correctly. More patches coming shortly.
>
> -Emil


// Tapani