Re: [dpdk-dev] [PATCH] eal: fix API to get error string

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

On 11/2/2018 9:51 AM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Wed, 31 Oct 2018 17:19:28 +0000
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> To: Bruce Richardson <bruce.richardson@intel.com>
>> CC: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>, stable@dpdk.org
>> Subject: [dpdk-dev] [PATCH] eal: fix API to get error string
>> X-Mailer: git-send-email 2.17.2
>>
>> External Email
>>
>> rte_strerror uses strerror_r(), and strerror_r() has two version of it.
>> - XSI-compliant version, (_POSIX_C_SOURCE >= 200112L) && !  _GNU_SOURCE
>> - GNU-specific version
>>
>> Those two has different return types, so the exiting return type check
>> is not correct for GNU-specific version.
>>
>> And this is causing failure in errno_autotest unit test.
>>
>> Adding different implementation for FreeBSD and Linux.
>>
>> Fixes: 016c32bd3e3d ("eal: cleanup strerror function")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>  lib/librte_eal/common/eal_common_errno.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/eal_common_errno.c b/lib/librte_eal/common/eal_common_errno.c
>> index 56b492f5f..fbbc71b0b 100644
>> --- a/lib/librte_eal/common/eal_common_errno.c
>> +++ b/lib/librte_eal/common/eal_common_errno.c
>> @@ -38,9 +38,17 @@ rte_strerror(int errnum)
>>                 case E_RTE_NO_CONFIG:
>>                         return "Missing rte_config structure";
>>                 default:
>> +#ifdef RTE_EXEC_ENV_BSDAPP
>>                         if (strerror_r(errnum, ret, RETVAL_SZ) != 0)
>>                                 snprintf(ret, RETVAL_SZ, "Unknown error%s %d",
>>                                                 sep, errnum);
>> +#else
>> +                       /*
>> +                        * _GNU_SOURCE version, error string is not always
>> +                        * strored in "ret" buffer, need to use return value
>> +                        */
>> +                       ret = strerror_r(errnum, ret, RETVAL_SZ);
> 
> Probably this will fail in musl c version.
> https://git.musl-libc.org/cgit/musl/tree/src/string/strerror_r.c

You are right, it will fail with musl. It may not be good idea to separate this
as BSD and Linux.

Instead of playing with strerror_r(), what about use strerror() and copy string
to RTE_PER_LCORE(retval)? I will send a patch for it.

> 
> Another alternative of this patch.
> 
> http://patches.dpdk.org/patch/47706/

I think this works, but I am not sure if it will have any side effect. And if we
want to add more functions to this file, that may be effected. I am more to fix
this locally in rte_strerror() function.

> 
>> +#endif
>>                 }
>>
>>         return ret;
>> --
>> 2.17.2
>>