19/11/2021 10:34, Ferruh Yigit: > On 11/18/2021 8:19 PM, Tyler Retzlaff wrote: > > On Thu, Nov 18, 2021 at 07:28:02PM +0000, [email protected] wrote: > >> From: Elena Agostini <[email protected]> > >> > >> Signed-off-by: Elena Agostini <[email protected]> > >> --- > >> 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
Which function returns a positive errno? > > 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. I think this patch is following the best practice. 1/ Return negative value in case of error 2/ Set rte_errno 3/ Set same absolute value in rte_errno and return code
