On 05/09/2018 04:02 AM, Shaikh, Shahed wrote:
-----Original Message-----
From: dev <dev-boun...@dpdk.org> On Behalf Of Bruce Richardson
Sent: Tuesday, May 8, 2018 2:53 PM
To: dev-boun...@dpdk.org
Cc: Andy Green <a...@warmcat.com>; 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-boun...@dpdk.org wrote:
-----Original Message-----
From: dev <dev-boun...@dpdk.org> 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
