[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 <elongbug at gmail.com> wrote:

> This patch adds missing error-checking and fixes resource leak in
> allocation failure path on anv_CreateDevice()
>
> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
> ---
>  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(&device->mutex, NULL);
> +   if (pthread_mutex_init(&device->mutex, NULL) != 0) {
> +      result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +      goto fail_context_id;
> +   }
>
>     pthread_condattr_t condattr;
> -   pthread_condattr_init(&condattr);
> -   pthread_condattr_setclock(&condattr, CLOCK_MONOTONIC);
> -   pthread_cond_init(&device->queue_submit, NULL);
> +   if (pthread_condattr_init(&condattr) != 0) {
> +      result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +      goto fail_mutex;
> +   }
> +   if (pthread_condattr_setclock(&condattr, CLOCK_MONOTONIC) != 0) {
> +      pthread_condattr_destroy(&condattr);
> +      result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +      goto fail_mutex;
> +   }
> +   if (pthread_cond_init(&device->queue_submit, NULL) != 0) {
> +      pthread_condattr_destroy(&condattr);
> +      result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
> +      goto fail_mutex;
> +   }
>     pthread_condattr_destroy(&condattr);
>
>     anv_bo_pool_init(&device->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(&device->dynamic_state_block_pool, device, 16384);
> +   result = anv_block_pool_init(&device->dynamic_state_block_pool,
> device,
> +                                16384);
> +   if (result != VK_SUCCESS)
> +      goto fail_queue_submit;
>
>     anv_state_pool_init(&device->dynamic_state_pool,
>                         &device->dynamic_state_block_pool);
>
> -   anv_block_pool_init(&device->instruction_block_pool, device, 128 *
> 1024);
> +   result = anv_block_pool_init(&device->instruction_block_pool, device,
> +                                128 * 1024);
> +   if (result != VK_SUCCESS)
> +      goto fail_dynamic_state_block_pool;
> +
>     anv_state_pool_init(&device->instruction_state_pool,
>                         &device->instruction_block_pool);
>
> -   anv_block_pool_init(&device->surface_state_block_pool, device, 4096);
> +   result = anv_block_pool_init(&device->surface_state_block_pool,
> device,
> +                                4096);
> +   if (result != VK_SUCCESS)
> +      goto fail_instruction_block_pool;
>
>     anv_state_pool_init(&device->surface_state_pool,
>                         &device->surface_state_block_pool);
>
> -   anv_bo_init_new(&device->workaround_bo, device, 1024);
> +   result = anv_bo_init_new(&device->workaround_bo, device, 1024);
> +   if (result != VK_SUCCESS)
> +      goto fail_surface_state_block_pool;
>
>     anv_scratch_pool_init(device, &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(&device->surface_state_block_pool);
> + fail_instruction_block_pool:
> +   anv_block_pool_finish(&device->instruction_block_pool);
> + fail_dynamic_state_block_pool:
> +   anv_block_pool_finish(&device->dynamic_state_block_pool);
> + fail_queue_submit:
> +   pthread_cond_destroy(&device->queue_submit);
> + fail_mutex:
> +   pthread_mutex_destroy(&device->mutex);
> + fail_context_id:
> +   anv_gem_destroy_context(device, device->context_id);
>   fail_fd:
>     close(device->fd);
>   fail_device:
> --
> 2.10.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161125/4a79e54a/attachment-0001.html>