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

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 11/18/2021 8:19 PM, Tyler Retzlaff wrote:
> 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 is a generic comment, cc'ed a few more folks to make the comment more
visible.

> 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!
>