On 09/09/2015 10:59 AM, Emil Velikov wrote:
> On 9 September 2015 at 01:43, Ian Romanick <i...@freedesktop.org> wrote:
>> On 09/07/2015 01:58 AM, Emil Velikov wrote:
>>> From: Matt Turner <matts...@gmail.com>
>>>
>>> v2: [Emil Velikov]
>>> Rework the error path to a common goto, close only if we own the fd.
>>>
>>> Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com>
>>> ---
>>> src/egl/drivers/dri2/platform_drm.c | 27 ++++++++++++++-------------
>>> 1 file changed, 14 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/egl/drivers/dri2/platform_drm.c
>>> b/src/egl/drivers/dri2/platform_drm.c
>>> index eda5087..e8fe7ea 100644
>>> --- a/src/egl/drivers/dri2/platform_drm.c
>>> +++ b/src/egl/drivers/dri2/platform_drm.c
>>> @@ -623,26 +623,20 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay
>>> *disp)
>>> dri2_dpy->own_device = 1;
>>> gbm = gbm_create_device(fd);
>>> if (gbm == NULL)
>>> - return EGL_FALSE;
>>> + goto cleanup;
>>> }
>>>
>>> - if (strcmp(gbm_device_get_backend_name(gbm), "drm") != 0) {
>>> - free(dri2_dpy);
>>> - return EGL_FALSE;
>>> - }
>>> + if (strcmp(gbm_device_get_backend_name(gbm), "drm") != 0)
>>> + goto cleanup;
>>>
>>> dri2_dpy->gbm_dri = gbm_dri_device(gbm);
>>> - if (dri2_dpy->gbm_dri->base.type != GBM_DRM_DRIVER_TYPE_DRI) {
>>> - free(dri2_dpy);
>>> - return EGL_FALSE;
>>> - }
>>> + if (dri2_dpy->gbm_dri->base.type != GBM_DRM_DRIVER_TYPE_DRI)
>>> + goto cleanup;
>>>
>>> if (fd < 0) {
>>> fd = fcntl(gbm_device_get_fd(gbm), F_DUPFD_CLOEXEC, 3);
>>> - if (fd < 0) {
>>> - free(dri2_dpy);
>>> - return EGL_FALSE;
>>> - }
>>> + if (fd < 0)
>>> + goto cleanup;
>>
>> Shouldn't this fd also get closed eventually? If we decide to catch
>> other failures (e.g., memory allocation failures) later in this function?
>>
> By 'this fd' I assume you mean the dup'd fd as opposed to the
> device_fd ? Yes we should, Matt's patch was doing the correct thing as
> I misread the DUPFD part in F_DUPFD_CLOEXEC :-\
Yeah, that's what I meant.
In spite of that, I like the 'goto cleanup' change. I was going to
suggest that to Matt... but you went and did it before I could suggest
it. :) Maybe blend the strengths?
> Thanks
> Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
