[Mesa-dev] [PATCH 2/3] egl/x11: set EGL_BAD_NATIVE_(PIXMAP|WINDOW) for invalid pixmaps/windows

[Frank Binns]

On 03/08/15 13:28, Emil Velikov wrote:
> On 31 July 2015 at 11:02, Frank Binns <frank.binns at imgtec.com> wrote:
>> Both eglCreatePixmapSurface and eglCreateWindowSurface were incorrectly
>> setting the EGL error to be EGL_BAD_ALLOC when an invalid native drawable
>> handle was being passed in. The EGL spec states the following for
>> eglCreatePixmapSurface:
>>
>>         "If pixmap is not a valid native pixmap handle, then an EGL_BAD_-
>>          NATIVE_PIXMAP error should be generated."
>>
>> (eglCreateWindowSurface has similar text)
>>
>> Correctly set the EGL error value based on xcb_get_geometry_reply returning
>> an error structure.
>>
>> Signed-off-by: Frank Binns <frank.binns at imgtec.com>
>> ---
>>  src/egl/drivers/dri2/platform_x11.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
>> index eb8d185..d35e9e2 100644
>> --- a/src/egl/drivers/dri2/platform_x11.c
>> +++ b/src/egl/drivers/dri2/platform_x11.c
>> @@ -265,10 +265,16 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>>     if (type != EGL_PBUFFER_BIT) {
>>        cookie = xcb_get_geometry (dri2_dpy->conn, dri2_surf->drawable);
>>        reply = xcb_get_geometry_reply (dri2_dpy->conn, cookie, &error);
>> -      if (reply == NULL || error != NULL) {
>> -        _eglError(EGL_BAD_ALLOC, "xcb_get_geometry");
>> -        free(error);
>> -        goto cleanup_dri_drawable;
>> +      if (error != NULL) {
>> +         if (type == EGL_WINDOW_BIT)
>> +            _eglError(EGL_BAD_NATIVE_WINDOW, "xcb_get_geometry");
>> +         else
>> +            _eglError(EGL_BAD_NATIVE_PIXMAP, "xcb_get_geometry");
> Obviously in most cases xcb_get_geometry_reply will fail due to the
> above two reasons, but won't these trigger even if we fail due to
> ENOMEM ? The xcb documentation does not explicitly mention if error
> will be non-NULL in the latter case, although common sense seems to
> hint that way.
>
> -Emil
>From what I can see both 'reply' and 'error' will be NULL if it fails on
the client side, in which case it will correctly set EGL_BAD_ALLOC. On
the server side it doesn't look like it can fail with ENOMEM/BadAlloc
but I'll update the patch to check for it just in case.

Thanks
Frank