[Beignet] [PATCH] Refine error check in clCreateCommandQueue/clCreateKernel

[Zhigang Gong]

On Tue, May 28, 2013 at 09:30:33AM +0800, Ruiling Song wrote:

I have one thought here. Can we put all these parameter checking into
cl_api.c which is in the entry functions of our library. Thus we can
get the following two advantages:

1. Concentrate most of the CL level sainty check into one place and easily to maintain.
2. Easily avoid duplicate error checking in multiple places.
3. A little bit short code path if it fails the sanity check :).

Any comments?

> Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> ---
>  src/cl_context.c |   13 ++++++++++++-
>  src/cl_program.c |    2 +-
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cl_context.c b/src/cl_context.c
> index fc75d62..8bbcdf5 100644
> --- a/src/cl_context.c
> +++ b/src/cl_context.c
> @@ -216,6 +216,16 @@ cl_context_create_queue(cl_context ctx,
>      goto error;
>    }
>  
> +  if(UNLIKELY(properties & ~(CL_QUEUE_OUT_OF_ORDER_EXEC_MODE_ENABLE | CL_QUEUE_PROFILING_ENABLE))) {
> +    err = CL_INVALID_VALUE;
> +    goto error;
> +  }
> +
> +  if(properties) {
> +    err = CL_INVALID_QUEUE_PROPERTIES;
> +    goto error;
> +  }
> +
>    /* We create the command queue and store it in the context list of queues */
>    TRY_ALLOC (queue, cl_command_queue_new(ctx));
>  
> @@ -224,7 +234,8 @@ exit:
>      *errcode_ret = err;
>    return queue;
>  error:
> -  cl_command_queue_delete(queue);
> +  if(queue)
> +    cl_command_queue_delete(queue);
>    queue = NULL;
>    goto exit;
>  }
> diff --git a/src/cl_program.c b/src/cl_program.c
> index 0c48ef3..0db2c97 100644
> --- a/src/cl_program.c
> +++ b/src/cl_program.c
> @@ -298,7 +298,7 @@ cl_program_create_kernel(cl_program p, const char *name, cl_int *errcode_ret)
>    uint32_t i = 0;
>  
>    if (UNLIKELY(name == NULL)) {
> -    err = CL_INVALID_KERNEL_NAME;
> +    err = CL_INVALID_VALUE;
>      goto error;
>    }
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet