[Mesa-dev] [PATCH] squash! anv: Move close(fd) from anv_bo_cache_import to its callers

[Chad Versace]

On Mon 16 Oct 2017, Jason Ekstrand wrote:
> On October 16, 2017 4:18:45 PM Chad Versace <chadversary at chromium.org> wrote:
> 
> > 
> > Add missing close(fd) for case
> > VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR,
> > subcase ANV_SEMAPHORE_TYPE_BO.
> > ---
> >  src/intel/vulkan/anv_queue.c | 22 +++++++++++-----------
> >  1 file changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
> > index e26254a87e..180c907781 100644
> > --- a/src/intel/vulkan/anv_queue.c
> > +++ b/src/intel/vulkan/anv_queue.c
> > @@ -1020,17 +1020,6 @@ VkResult anv_ImportSemaphoreFdKHR(
> >           new_impl.syncobj = anv_gem_syncobj_fd_to_handle(device, fd);
> >           if (!new_impl.syncobj)
> >              return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR);
> > -
> > -         /* From the Vulkan spec:
> > -          *
> > -          *    "Importing semaphore state from a file descriptor transfers
> > -          *    ownership of the file descriptor from the application to the
> > -          *    Vulkan implementation. The application must not perform any
> > -          *    operations on the file descriptor after a successful import."
> > -          *
> > -          * If the import fails, we leave the file descriptor open.
> > -          */
> > -         close(pImportSemaphoreFdInfo->fd);
> >        } else {
> >           new_impl.type = ANV_SEMAPHORE_TYPE_BO;
> > 
> > @@ -1044,6 +1033,17 @@ VkResult anv_ImportSemaphoreFdKHR(
> >            */
> >           assert(!(new_impl.bo->flags & EXEC_OBJECT_ASYNC));
> >        }
> > +
> > +      /* From the Vulkan spec:
> > +       *
> > +       *    "Importing semaphore state from a file descriptor transfers
> > +       *    ownership of the file descriptor from the application to the
> > +       *    Vulkan implementation. The application must not perform any
> > +       *    operations on the file descriptor after a successful import."
> > +       *
> > +       * If the import fails, we leave the file descriptor open.
> > +       */
> > +      close(fd);
> 
> Indentation looks off here.  Other than that, LGTM

The indentation is intended. The expanded hunk looks like this. The
close() catches both branches of the if-tree.

    case VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR:
        if (has_syncobj) {
            ...
            anv_gem_syncobj_fd_to_handle(dev, fd);
            if (fail)
                return error;
        } else {
            ...
            anv_bo_cache_import_with_size(dev, fd, ...);
            if (fail)
                return error;
        }

        close(fd);