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

[Frank Binns]

On 03/08/15 13:05, Emil Velikov wrote:
> 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
Sure, I've updated the commit message and I'll resend it a bit later.

Thanks
Frank