[Mesa-dev] [PATCH 1/3] egl/x11: fix use of EGL_BAD_NATIVE_WINDOW

[Emil Velikov]

On 31 July 2015 at 13:34, Frank Binns <frank.binns at imgtec.com> wrote:
> On 31/07/15 12:53, Emil Velikov wrote:
>> On 31 July 2015 at 11:02, Frank Binns <frank.binns at imgtec.com> wrote:
>>> Commit 4ed23fd590 introduced some calls to _eglError inappropriately
>>> passing it EGL_BAD_NATIVE_WINDOW. This was actually harmless in two of the
>>> cases as _eglError gets called later on with a more appropriate error code
>>> but (just to be safe) switch these to _eglLog calls instead. In the
>>> remaining case, change the error to EGL_BAD_ALLOC instead as this seems
>>> more appropriate.
>>>
>> Hmm... EGL_BAD_NATIVE_WINDOW might not be the best, but EGL_BAD_ALLOC
>> doesn't seem much better.
>>
>> There is no memory allocation happening there, so I'm not sure why you
>> believe it more appropriate. I'm wondering about EGL_BAD_PARAMETER,
>> yet it's very unlikely we'll ever get here. Did you see any uses that
>> trigger this ?
>>
>> -Emil
> The errors available to us, i.e. the ones mentioned in the EGL spec, are:
> - EGL_BAD_MATCH
> - EGL_BAD_CONFIG
> - EGL_BAD_NATIVE_WINDOW
> - EGL_BAD_ALLOC
>
>From a closer look the function is used via
eglInitialize (we're ok at auth, but we'll crash in
dri2_x11_add_configs_for_visuals)
eglCreate*Surface

And ... EGL_BAD_PARAMETER is not available in either one.

> I believe there are two possible ways we can end up going down this path:
> 1) xcb_get_setup fails due to a previous error on the connection. The
> error that appears most in the xcb code is XCB_CONN_CLOSED_MEM_INSUFFICIENT.
>
> 2) The 'rem' field of xcb_screen_iterator_t is 0.
>
> In both cases EGL_BAD_ALLOC seems like the most appropriate error.
>
> I've not actually hit this error path but spotted it when making the
> fixes in patches 2 and 3.
>
Thank you for the explanation, I've rarely had the pleasure to go
though the xcb code.
Can you add some of this information in the commit message without
that knowledge the patch looks rather odd.

Cheers,
Emil