From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] eal: fix API to get error string Date: Fri, 2 Nov 2018 17:00:01 +0000 Message-ID: References: <20181031171928.61110-1-ferruh.yigit@intel.com> <20181102095046.GA32112@jerin> <6b854290-7835-4a05-1273-ddab0b740957@intel.com> <20181102154517.GA20440@jerin> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Bruce Richardson , "dev@dpdk.org" , "stable@dpdk.org" To: Jerin Jacob Return-path: In-Reply-To: <20181102154517.GA20440@jerin> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11/2/2018 3:45 PM, Jerin Jacob wrote: > -----Original Message----- >> Date: Fri, 2 Nov 2018 15:39:04 +0000 >> From: Ferruh Yigit >> To: Jerin Jacob >> CC: Bruce Richardson , "dev@dpdk.org" >> , "stable@dpdk.org" >> Subject: Re: [dpdk-dev] [PATCH] eal: fix API to get error string >> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 >> Thunderbird/52.9.1 >> >> >> On 11/2/2018 9:51 AM, Jerin Jacob wrote: >>> -----Original Message----- >>>> Date: Wed, 31 Oct 2018 17:19:28 +0000 >>>> From: Ferruh Yigit >>>> To: Bruce Richardson >>>> CC: dev@dpdk.org, Ferruh Yigit , 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 >>>> --- >>>> 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. > > I thought we used strerror_r() to enable thread safety.IMO, strerror() > is not thread safe. > >> >>> >>> 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. > > If we can then it is good. I was hoping to use "#if !defined(_GNU_SOURCE)" to differentiate glibc and musl, but we can't because gcc/clang adds this macro themselves. +1 to your fix. We can dedicate this .c file only for this API at worst. > >> >>> >>>> +#endif >>>> } >>>> >>>> return ret; >>>> -- >>>> 2.17.2 >>>> >>