From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.warmcat.com (mail.warmcat.com [163.172.24.82]) by dpdk.org (Postfix) with ESMTP id 212277CFD; Wed, 9 May 2018 00:08:14 +0200 (CEST) To: "Shaikh, Shahed" , Bruce Richardson , "dev-bounces@dpdk.org" Cc: "dev@dpdk.org" References: <152575364588.56689.3300796065057551728.stgit@localhost.localdomain> <152575381332.56689.17827274556476490336.stgit@localhost.localdomain> <20180508195315.GB18108@bricha3-MOBL.ger.corp.intel.com> From: Andy Green Message-ID: Date: Wed, 9 May 2018 06:07:09 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 May 2018 22:08:15 -0000 On 05/09/2018 04:02 AM, Shaikh, Shahed wrote: >> -----Original Message----- >> From: dev On Behalf Of Bruce Richardson >> Sent: Tuesday, May 8, 2018 2:53 PM >> To: dev-bounces@dpdk.org >> Cc: Andy Green ; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant >> and NUL >> >> On Tue, May 08, 2018 at 05:59:47PM +0000, dev-bounces@dpdk.org wrote: >>> >>> >>>> -----Original Message----- >>>> From: dev On Behalf Of Andy Green >>>> Sent: Monday, May 7, 2018 11:30 PM >>>> To: dev@dpdk.org >>>> Subject: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy >>>> constant and NUL >>>> >>>> >>>> --- >>>> drivers/net/qede/base/ecore_int.c | 10 ++++++---- >>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/net/qede/base/ecore_int.c >>>> b/drivers/net/qede/base/ecore_int.c >>>> index f43781ba4..c809d84ef 100644 >>>> --- a/drivers/net/qede/base/ecore_int.c >>>> +++ b/drivers/net/qede/base/ecore_int.c >>>> @@ -1103,10 +1103,12 @@ static enum _ecore_status_t >>>> ecore_int_deassertion(struct ecore_hwfn *p_hwfn, >>>> OSAL_SNPRINTF(bit_name, 30, >>>> p_aeu->bit_name, >>>> num); >>>> - else >>>> - OSAL_STRNCPY(bit_name, >>>> - p_aeu->bit_name, >>>> - 30); >>>> + else { >>>> + strncpy(bit_name, >>>> + p_aeu->bit_name, >>>> + sizeof(bit_name) - 1); >>>> + bit_name[sizeof(bit_name) - 1] >>>> = '\0'; >>>> + } >>> >>> I think you can retain OSAL_STRNCPY and just replace 30 with >> 'bit_name[sizeof(bit_name) - 1' and then set last byte with '\0' just like you did. >> >> Can that actually be fixed inside OSAL_STRNCPY itself, rather than having each >> use needing to explicitly null-terminate? > > Although there is only instance of OSAL_STRNCPY, it makes sense to modify it. Doesn't it make more sense to get rid of OSAL_* that bring nothing at all to the party? #define OSAL_SPRINTF(name, pattern, ...) \ sprintf(name, pattern, ##__VA_ARGS__) #define OSAL_SNPRINTF(buf, size, format, ...) \ snprintf(buf, size, format, ##__VA_ARGS__) #define OSAL_STRLEN(string) strlen(string) #define OSAL_STRCPY(dst, string) strcpy(dst, string) #define OSAL_STRNCPY(dst, string, len) strncpy(dst, string, len) #define OSAL_STRCMP(str1, str2) strcmp(str1, str2) Do I miss the point or these are just cruft? -Andy > Thanks, > Shahed >