<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Oct 17, 2017 at 10:46 AM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon 16 Oct 2017, Jason Ekstrand wrote:<br>
> On October 16, 2017 4:18:45 PM Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>> wrote:<br>
><br>
> ><br>
> > Add missing close(fd) for case<br>
> > VK_EXTERNAL_SEMAPHORE_HANDLE_<wbr>TYPE_OPAQUE_FD_BIT_KHR,<br>
> > subcase ANV_SEMAPHORE_TYPE_BO.<br>
> > ---<br>
> > src/intel/vulkan/anv_queue.c | 22 +++++++++++-----------<br>
> > 1 file changed, 11 insertions(+), 11 deletions(-)<br>
> ><br>
> > diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c<br>
> > index e26254a87e..180c907781 100644<br>
> > --- a/src/intel/vulkan/anv_queue.c<br>
> > +++ b/src/intel/vulkan/anv_queue.c<br>
> > @@ -1020,17 +1020,6 @@ VkResult anv_ImportSemaphoreFdKHR(<br>
> > new_impl.syncobj = anv_gem_syncobj_fd_to_handle(<wbr>device, fd);<br>
> > if (!new_impl.syncobj)<br>
> > return vk_error(VK_ERROR_INVALID_<wbr>EXTERNAL_HANDLE_KHR);<br>
> > -<br>
> > - /* From the Vulkan spec:<br>
> > - *<br>
> > - * "Importing semaphore state from a file descriptor transfers<br>
> > - * ownership of the file descriptor from the application to the<br>
> > - * Vulkan implementation. The application must not perform any<br>
> > - * operations on the file descriptor after a successful import."<br>
> > - *<br>
> > - * If the import fails, we leave the file descriptor open.<br>
> > - */<br>
> > - close(pImportSemaphoreFdInfo-><wbr>fd);<br>
> > } else {<br>
> > new_impl.type = ANV_SEMAPHORE_TYPE_BO;<br>
> ><br>
> > @@ -1044,6 +1033,17 @@ VkResult anv_ImportSemaphoreFdKHR(<br>
> > */<br>
> > assert(!(new_impl.bo->flags & EXEC_OBJECT_ASYNC));<br>
> > }<br>
> > +<br>
> > + /* From the Vulkan spec:<br>
> > + *<br>
> > + * "Importing semaphore state from a file descriptor transfers<br>
> > + * ownership of the file descriptor from the application to the<br>
> > + * Vulkan implementation. The application must not perform any<br>
> > + * operations on the file descriptor after a successful import."<br>
> > + *<br>
> > + * If the import fails, we leave the file descriptor open.<br>
> > + */<br>
> > + close(fd);<br>
><br>
> Indentation looks off here. Other than that, LGTM<br>
<br>
</div></div>The indentation is intended. The expanded hunk looks like this. The<br>
close() catches both branches of the if-tree.<br>
<br>
case VK_EXTERNAL_SEMAPHORE_HANDLE_<wbr>TYPE_OPAQUE_FD_BIT_KHR:<br>
if (has_syncobj) {<br>
...<br>
anv_gem_syncobj_fd_to_handle(<wbr>dev, fd);<br>
if (fail)<br>
return error;<br>
} else {<br>
...<br>
anv_bo_cache_import_with_size(<wbr>dev, fd, ...);<br>
if (fail)<br>
return error;<br>
}<br>
<br>
close(fd);<br></blockquote><div> </div></div></div><div class="gmail_extra">Yes, but it looked, in patchwork, like it was indented 1 space too far.<br></div></div>