Re: [PATCH v1] gpudev: return EINVAL if invalid input pointer for free and unregister

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Thu, Nov 18, 2021 at 07:28:02PM +0000, eagostini@nvidia.com wrote:
> From: Elena Agostini <eagostini@nvidia.com>
> 
> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> ---
>  lib/gpudev/gpudev.c     | 10 ++++++++++
>  lib/gpudev/rte_gpudev.h |  2 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/lib/gpudev/gpudev.c b/lib/gpudev/gpudev.c
> index 2b174d8bd5..97575ed979 100644
> --- a/lib/gpudev/gpudev.c
> +++ b/lib/gpudev/gpudev.c
> @@ -576,6 +576,11 @@ rte_gpu_mem_free(int16_t dev_id, void *ptr)
>  		return -rte_errno;
>  	}
>  
> +	if (ptr == NULL) {
> +		rte_errno = EINVAL;
> +		return -rte_errno;
> +	}

in general dpdk has real problems with how it indicates that an error
occurred and what error occurred consistently.

some api's return 0 on success
  and maybe return -errno if ! 0
  and maybe return errno if ! 0
  and maybe set rte_errno if ! 0

some api's return -1 on failure
  and set rte_errno if -1

some api's return < 0 on failure
  and maybe set rte_errno
  and maybe return -errno
  and maybe set rte_errno and return -rte_errno

this isn't isiolated to only this change but since additions and context
in this patch highlight it maybe it's a good time to bring it up.

it's frustrating to have to carefully read the implementation every time
you want to make a function call to make sure you're handling the flavor
of error reporting for a particular function.

if this is new code could we please clearly identify the current best
practice and follow it as a standard going forward for all new public
apis.

thanks!