Re: [PATCH v3 0/2] Consolidate create-sync and create-fence

["Kim, Dongwon" <dongwon.kim@intel.com>]

Hey Marc-André,

On 7/24/2024 3:37 AM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 24, 2024 at 2:05 AM <dongwon.kim@intel.com 
> <mailto:dongwon.kim@intel.com>> wrote:
> 
>     From: Dongwon Kim <dongwon.kim@intel.com <mailto:dongwon.kim@intel.com>>
> 
>     Sync object itself is never used as is so can be removed
>     from QemuDmaBuf struct. So now sync is only temporarily needed
>     when creating fence for the object which means what was done in
>     egl_dmabuf_create_sync can now be a part of egl_dmabuf_create_fence
>     function. And egl_dmabuf_create_fence returns fence_fd so the
>     better function name will be egl_dmabuf_create_fence_fd.
> 
>     v3: create fence only if current QemuDmaBuf->fence_fd = -1
>          to make sure there is no fence currently bound to the
>          QemuDmaBuf
> 
> 
> Why not check it from egl_dmabuf_create_fence_fd() ? calling the 
> function twice can still potentially leak.

It is called from only two locations in gtk-egl.c and gtk-gl-draw.c
and dmabuf->fence_fd == -1 is checked beforehand so I thought it's
not needed but I think your point is the completeness of the function
itself. Do you think we can do assert 'dmabuf->fence_fd >= 0'?

> 
> Also, please gather the v1/v2/v3/... summary on the cover letter.

Sure

> 
> thanks
> 
> 
>     Dongwon Kim (2):
>        ui/egl-helpers: Consolidates create-sync and create-fence
>        ui/dmabuf: Remove 'sync' from QemuDmaBuf struct
> 
>       include/ui/dmabuf.h      |  2 --
>       include/ui/egl-helpers.h |  3 +--
>       ui/dmabuf.c              | 14 --------------
>       ui/egl-helpers.c         | 24 +++++++++---------------
>       ui/gtk-egl.c             | 17 ++++-------------
>       ui/gtk-gl-area.c         | 12 +++---------
>       6 files changed, 17 insertions(+), 55 deletions(-)
> 
>     -- 
>     2.43.0
> 
> 
> 
> 
> -- 
> Marc-André Lureau