[Mesa-dev] [PATCH 3/3] egl/x11: don't abort when creating a DRI2 drawable fails

[Frank Binns]

On 03/08/15 13:33, Emil Velikov wrote:
> On 31 July 2015 at 11:02, Frank Binns <frank.binns at imgtec.com> wrote:
>> When calling either eglCreateWindowSurface or eglCreatePixmapSurface it
>> was possible for an application to be aborted as a result of it failing
>> to create a DRI2 drawable on the server. This could happen due to an
>> application passing in an invalid native drawable handle, for example.
>>
>> Signed-off-by: Frank Binns <frank.binns at imgtec.com>
>> ---
>>  src/egl/drivers/dri2/platform_x11.c | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
>> index d35e9e2..830e643 100644
>> --- a/src/egl/drivers/dri2/platform_x11.c
>> +++ b/src/egl/drivers/dri2/platform_x11.c
>> @@ -284,7 +284,23 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>>     }
>>
>>     if (dri2_dpy->dri2) {
>> -      xcb_dri2_create_drawable (dri2_dpy->conn, dri2_surf->drawable);
>> +      xcb_void_cookie_t cookie;
>> +
>> +      cookie = xcb_dri2_create_drawable_checked(dri2_dpy->conn,
>> +                                                dri2_surf->drawable);
>> +      error = xcb_request_check(dri2_dpy->conn, cookie);
>> +      if (error != NULL) {
>> +         if (error->error_code == BadAlloc || type == EGL_PBUFFER_BIT)
>> +            _eglError(EGL_BAD_ALLOC, "xcb_dri2_create_drawable_checked");
>> +         else if (type == EGL_WINDOW_BIT)
>> +            _eglError(EGL_BAD_NATIVE_WINDOW,
>> +                      "xcb_dri2_create_drawable_checked");
>> +         else
>> +            _eglError(EGL_BAD_NATIVE_PIXMAP,
>> +                      "xcb_dri2_create_drawable_checked");
> Similar to previous commit - shouldn't one check for BadAlloc prior to
> these two ?
>
> Would be great if we get another pair of eyes looking this way though
> - XCB is not my forte. I wonder if we should pick this (and? patch 2)
> for stable - crashing is never a good idea.
>
> Thanks
> Emil
It looks like xcb_request_check will return NULL if there was a previous
error on the connection, e.g. it ran out of memory. I'll update the
patch to deal with this.

Thanks
Frank