[Mesa-dev] [PATCH 2/2] egl/dri2/drm: compact existing device mgmt

[Boyan Ding]

2015-09-07 16:58 GMT+08:00 Emil Velikov <emil.l.velikov at gmail.com>:
> Move the fcntl(dupfd_cloexec) to the else branch where it belongs.
> Otherwise it's not immediately obvious that the code is hit, only when
> an existing device is used.

A potential problem here. The fd acquired from gbm device provided as
platform display has been dup'ed, so it should also be closed if we hit
the error condition below, regardless of own_device.

The first patch itself happened to be right because acquiring fd from
platform gbm device is the last place place where error can occur.

If we really want this one, we'd better close fd according to its
validity (you can change it in the first patch), i.e.

cleanup:
   if (dri2_dpy->fd >= 0)
      close(fd);

   free(dri2_dpy);
   return EGL_FALSE;

With that fixed, this patch is
Reviewed-by: Boyan Ding <boyan.j.ding at gmail.com>

>
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  src/egl/drivers/dri2/platform_drm.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
> index e8fe7ea..cccde12 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -624,6 +624,10 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
>        gbm = gbm_create_device(fd);
>        if (gbm == NULL)
>           goto cleanup;
> +   } else {
> +      fd = fcntl(gbm_device_get_fd(gbm), F_DUPFD_CLOEXEC, 3);
> +      if (fd < 0)
> +         goto cleanup;
>     }
>
>     if (strcmp(gbm_device_get_backend_name(gbm), "drm") != 0)
> @@ -633,12 +637,6 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
>     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)
> -         goto cleanup;
> -   }
> -
>     dri2_dpy->fd = fd;
>     dri2_dpy->device_name = loader_get_device_name_for_fd(dri2_dpy->fd);
>     dri2_dpy->driver_name = strdup(dri2_dpy->gbm_dri->base.driver_name);
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev