On Friday, 2017-06-30 12:15:11 +0100, Emil Velikov wrote: > From: Emil Velikov <emil.veli...@collabora.com> > > Don't bother allocating any memory until we're finished parsing and > sanitising all the attributes. > > As a nice side effect we now consistently set eglError when any of > the attrib/values are not correct.
Not initialising `err` would've caught this bug; please add the following hunk to this commit? ---8<--- diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index cf26242702..1ed511d6da 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -2298,7 +2298,7 @@ dri2_create_drm_image_mesa(_EGLDriver *drv, _EGLDisplay *disp, _EGLImageAttribs attrs; unsigned int dri_use, valid_mask; int format; - EGLint err = EGL_SUCCESS; + EGLint err; (void) drv; --->8--- Alternatively, you could move the call to _eglParseImageAttribList() inside the `if()` and get rid or `err` altogether. > > Strangely enough the spec does not mention _anything_ about what error > should be set where, even if the implementation already sets the odd > one. > > Cc: Kristian Høgsberg <k...@bitplanet.net> > Signed-off-by: Emil Velikov <emil.veli...@collabora.com> > --- > Kristian can you shed some light? The extension seems quite sparse. > > Weston used the extension back in 2011. While the Glamor bit were > dropped somewhat recently in May 2017. > > Other than those I cannot find any users of the extension - there's no > piglit tests even :-\ > > A crazy idea: Should we deprecate the extension? > --- > src/egl/drivers/dri2/egl_dri2.c | 52 > ++++++++++++++++++----------------------- > 1 file changed, 23 insertions(+), 29 deletions(-) > > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c > index cf262427020..e55bff6dbbf 100644 > --- a/src/egl/drivers/dri2/egl_dri2.c > +++ b/src/egl/drivers/dri2/egl_dri2.c > @@ -2302,27 +2302,20 @@ dri2_create_drm_image_mesa(_EGLDriver *drv, > _EGLDisplay *disp, > > (void) drv; > > - dri2_img = malloc(sizeof *dri2_img); > - if (!dri2_img) { > - _eglError(EGL_BAD_ALLOC, "dri2_create_image_khr"); > - return EGL_NO_IMAGE_KHR; > - } > - > if (!attr_list) { > - err = EGL_BAD_PARAMETER; > - goto cleanup_img; > + _eglError(EGL_BAD_PARAMETER, __func__); > + return EGL_NO_IMAGE_KHR; > } > > - _eglInitImage(&dri2_img->base, disp); > - > err = _eglParseImageAttribList(&attrs, disp, attr_list); > - if (err != EGL_SUCCESS) > - goto cleanup_img; > + if (err != EGL_SUCCESS) { > + _eglError(EGL_BAD_PARAMETER, __func__); > + return EGL_NO_IMAGE_KHR; > + } > > if (attrs.Width <= 0 || attrs.Height <= 0) { > - _eglLog(_EGL_WARNING, "bad width or height (%dx%d)", > - attrs.Width, attrs.Height); > - goto cleanup_img; > + _eglError(EGL_BAD_PARAMETER, __func__); > + return EGL_NO_IMAGE_KHR; > } > > switch (attrs.DRMBufferFormatMESA) { > @@ -2330,9 +2323,8 @@ dri2_create_drm_image_mesa(_EGLDriver *drv, _EGLDisplay > *disp, > format = __DRI_IMAGE_FORMAT_ARGB8888; > break; > default: > - _eglLog(_EGL_WARNING, "bad image format value 0x%04x", > - attrs.DRMBufferFormatMESA); > - goto cleanup_img; > + _eglError(EGL_BAD_PARAMETER, __func__); > + return EGL_NO_IMAGE_KHR; > } > > valid_mask = > @@ -2340,9 +2332,8 @@ dri2_create_drm_image_mesa(_EGLDriver *drv, _EGLDisplay > *disp, > EGL_DRM_BUFFER_USE_SHARE_MESA | > EGL_DRM_BUFFER_USE_CURSOR_MESA; > if (attrs.DRMBufferUseMESA & ~valid_mask) { > - _eglLog(_EGL_WARNING, "bad image use bit 0x%04x", > - attrs.DRMBufferUseMESA & ~valid_mask); > - goto cleanup_img; > + _eglError(EGL_BAD_PARAMETER, __func__); > + return EGL_NO_IMAGE_KHR; > } > > dri_use = 0; > @@ -2353,22 +2344,25 @@ dri2_create_drm_image_mesa(_EGLDriver *drv, > _EGLDisplay *disp, > if (attrs.DRMBufferUseMESA & EGL_DRM_BUFFER_USE_CURSOR_MESA) > dri_use |= __DRI_IMAGE_USE_CURSOR; > > + dri2_img = malloc(sizeof *dri2_img); > + if (!dri2_img) { > + _eglError(EGL_BAD_ALLOC, "dri2_create_image_khr"); > + return EGL_NO_IMAGE_KHR; > + } > + > + _eglInitImage(&dri2_img->base, disp); > + > dri2_img->dri_image = > dri2_dpy->image->createImage(dri2_dpy->dri_screen, > attrs.Width, attrs.Height, > format, dri_use, dri2_img); > if (dri2_img->dri_image == NULL) { > - err = EGL_BAD_ALLOC; > - goto cleanup_img; > + free(dri2_img); > + _eglError(EGL_BAD_ALLOC, "dri2_create_drm_image_mesa"); > + return EGL_NO_IMAGE_KHR; > } > > return &dri2_img->base; > - > - cleanup_img: > - free(dri2_img); > - _eglError(err, "dri2_create_drm_image_mesa"); > - > - return EGL_NO_IMAGE_KHR; > } > > static EGLBoolean > -- > 2.13.0 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev