Re: [Mesa-dev] [PATCH 4/5] anv: Add missing error-checking to anv_CreateDevice

[Jason Ekstrand]

On Fri, Nov 25, 2016 at 6:34 AM, Mun Gwan-gyeong  wrote:

> This patch adds missing error-checking and fixes resource leak in
> allocation failure path on anv_CreateDevice()
>
> Signed-off-by: Mun Gwan-gyeong 
> ---
>  src/intel/vulkan/anv_device.c | 59 ++
> ++---
>  1 file changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 0fd7d41..1964fb7 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -893,31 +893,57 @@ VkResult anv_CreateDevice(
> device->robust_buffer_access = pCreateInfo->pEnabledFeatures &&
>pCreateInfo->pEnabledFeatures->robustBufferAccess;
>
> -   pthread_mutex_init(>mutex, NULL);
> +   if (pthread_mutex_init(>mutex, NULL) != 0) {
> +  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +  goto fail_context_id;
> +   }
>
> pthread_condattr_t condattr;
> -   pthread_condattr_init();
> -   pthread_condattr_setclock(, CLOCK_MONOTONIC);
> -   pthread_cond_init(>queue_submit, NULL);
> +   if (pthread_condattr_init() != 0) {
> +  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +  goto fail_mutex;
> +   }
> +   if (pthread_condattr_setclock(, CLOCK_MONOTONIC) != 0) {
> +  pthread_condattr_destroy();
> +  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +  goto fail_mutex;
> +   }
> +   if (pthread_cond_init(>queue_submit, NULL) != 0) {
> +  pthread_condattr_destroy();
> +  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +  goto fail_mutex;
> +   }
> pthread_condattr_destroy();
>
> anv_bo_pool_init(>batch_bo_pool, device);
>

You're missing the destructor for this below as well as destructors for all
of the state pools.  While I realize that those don't do anything
interesting if no states have been allocated yet, it'd be bets to include
them for completeness.


>
> -   anv_block_pool_init(>dynamic_state_block_pool, device, 16384);
> +   result = anv_block_pool_init(>dynamic_state_block_pool,
> device,
> +16384);
> +   if (result != VK_SUCCESS)
> +  goto fail_queue_submit;
>
> anv_state_pool_init(>dynamic_state_pool,
> >dynamic_state_block_pool);
>
> -   anv_block_pool_init(>instruction_block_pool, device, 128 *
> 1024);
> +   result = anv_block_pool_init(>instruction_block_pool, device,
> +128 * 1024);
> +   if (result != VK_SUCCESS)
> +  goto fail_dynamic_state_block_pool;
> +
> anv_state_pool_init(>instruction_state_pool,
> >instruction_block_pool);
>
> -   anv_block_pool_init(>surface_state_block_pool, device, 4096);
> +   result = anv_block_pool_init(>surface_state_block_pool,
> device,
> +4096);
> +   if (result != VK_SUCCESS)
> +  goto fail_instruction_block_pool;
>
> anv_state_pool_init(>surface_state_pool,
> >surface_state_block_pool);
>
> -   anv_bo_init_new(>workaround_bo, device, 1024);
> +   result = anv_bo_init_new(>workaround_bo, device, 1024);
> +   if (result != VK_SUCCESS)
> +  goto fail_surface_state_block_pool;
>
> anv_scratch_pool_init(device, >scratch_pool);
>
> @@ -942,7 +968,7 @@ VkResult anv_CreateDevice(
>unreachable("unhandled gen");
> }
> if (result != VK_SUCCESS)
> -  goto fail_fd;
> +  goto fail_workaround_bo;
>
> anv_device_init_blorp(device);
>
> @@ -952,6 +978,21 @@ VkResult anv_CreateDevice(
>
> return VK_SUCCESS;
>
> + fail_workaround_bo:
> +   anv_gem_munmap(device->workaround_bo.map, device->workaround_bo.size);
> +   anv_gem_close(device, device->workaround_bo.gem_handle);
> + fail_surface_state_block_pool:
> +   anv_block_pool_finish(>surface_state_block_pool);
> + fail_instruction_block_pool:
> +   anv_block_pool_finish(>instruction_block_pool);
> + fail_dynamic_state_block_pool:
> +   anv_block_pool_finish(>dynamic_state_block_pool);
> + fail_queue_submit:
> +   pthread_cond_destroy(>queue_submit);
> + fail_mutex:
> +   pthread_mutex_destroy(>mutex);
> + fail_context_id:
> +   anv_gem_destroy_context(device, device->context_id);
>   fail_fd:
> close(device->fd);
>   fail_device:
> --
> 2.10.2
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 4/5] anv: Add missing error-checking to anv_CreateDevice

[Mun Gwan-gyeong]

This patch adds missing error-checking and fixes resource leak in
allocation failure path on anv_CreateDevice()

Signed-off-by: Mun Gwan-gyeong 
---
 src/intel/vulkan/anv_device.c | 59 ---
 1 file changed, 50 insertions(+), 9 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 0fd7d41..1964fb7 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -893,31 +893,57 @@ VkResult anv_CreateDevice(
device->robust_buffer_access = pCreateInfo->pEnabledFeatures &&
   pCreateInfo->pEnabledFeatures->robustBufferAccess;
 
-   pthread_mutex_init(>mutex, NULL);
+   if (pthread_mutex_init(>mutex, NULL) != 0) {
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_context_id;
+   }
 
pthread_condattr_t condattr;
-   pthread_condattr_init();
-   pthread_condattr_setclock(, CLOCK_MONOTONIC);
-   pthread_cond_init(>queue_submit, NULL);
+   if (pthread_condattr_init() != 0) {
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_mutex;
+   }
+   if (pthread_condattr_setclock(, CLOCK_MONOTONIC) != 0) {
+  pthread_condattr_destroy();
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_mutex;
+   }
+   if (pthread_cond_init(>queue_submit, NULL) != 0) {
+  pthread_condattr_destroy();
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_mutex;
+   }
pthread_condattr_destroy();
 
anv_bo_pool_init(>batch_bo_pool, device);
 
-   anv_block_pool_init(>dynamic_state_block_pool, device, 16384);
+   result = anv_block_pool_init(>dynamic_state_block_pool, device,
+16384);
+   if (result != VK_SUCCESS)
+  goto fail_queue_submit;
 
anv_state_pool_init(>dynamic_state_pool,
>dynamic_state_block_pool);
 
-   anv_block_pool_init(>instruction_block_pool, device, 128 * 1024);
+   result = anv_block_pool_init(>instruction_block_pool, device,
+128 * 1024);
+   if (result != VK_SUCCESS)
+  goto fail_dynamic_state_block_pool;
+
anv_state_pool_init(>instruction_state_pool,
>instruction_block_pool);
 
-   anv_block_pool_init(>surface_state_block_pool, device, 4096);
+   result = anv_block_pool_init(>surface_state_block_pool, device,
+4096);
+   if (result != VK_SUCCESS)
+  goto fail_instruction_block_pool;
 
anv_state_pool_init(>surface_state_pool,
>surface_state_block_pool);
 
-   anv_bo_init_new(>workaround_bo, device, 1024);
+   result = anv_bo_init_new(>workaround_bo, device, 1024);
+   if (result != VK_SUCCESS)
+  goto fail_surface_state_block_pool;
 
anv_scratch_pool_init(device, >scratch_pool);
 
@@ -942,7 +968,7 @@ VkResult anv_CreateDevice(
   unreachable("unhandled gen");
}
if (result != VK_SUCCESS)
-  goto fail_fd;
+  goto fail_workaround_bo;
 
anv_device_init_blorp(device);
 
@@ -952,6 +978,21 @@ VkResult anv_CreateDevice(
 
return VK_SUCCESS;
 
+ fail_workaround_bo:
+   anv_gem_munmap(device->workaround_bo.map, device->workaround_bo.size);
+   anv_gem_close(device, device->workaround_bo.gem_handle);
+ fail_surface_state_block_pool:
+   anv_block_pool_finish(>surface_state_block_pool);
+ fail_instruction_block_pool:
+   anv_block_pool_finish(>instruction_block_pool);
+ fail_dynamic_state_block_pool:
+   anv_block_pool_finish(>dynamic_state_block_pool);
+ fail_queue_submit:
+   pthread_cond_destroy(>queue_submit);
+ fail_mutex:
+   pthread_mutex_destroy(>mutex);
+ fail_context_id:
+   anv_gem_destroy_context(device, device->context_id);
  fail_fd:
close(device->fd);
  fail_device:
-- 
2.10.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev