On Tue, Oct 17, 2017 at 10:46 AM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Mon 16 Oct 2017, Jason Ekstrand wrote:
> > On October 16, 2017 4:18:45 PM Chad Versace <chadvers...@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);
>
Yes, but it looked, in patchwork, like it was indented 1 space too far.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
