[dpdk-dev] [PATCH v3 0/2] gcc-8 build fixes

[dpdk-dev] [PATCH v3 0/2] gcc-8 build fixes

[Stephen Hemminger]

This fixes some of the obvious warnings found building DPDK
with gcc-8. There still are some deeper issues in the rte_hash_table
code; leave the fix for that up to the maintainer.

Stephen Hemminger (2):
  rte_mbuf: fix strncpy warnings
  rte_metrics: fix strncpy truncation warning

v3
  missing SOB on 1st patch

v2
  fix issues with wrong length in mbuf pool_ops
  don't need memset in metrics names

Stephen Hemminger (2):
  rte_mbuf: fix strncpy warnings
  rte_metrics: fix strncpy truncation warning

 lib/librte_mbuf/rte_mbuf_pool_ops.c | 4 ++--
 lib/librte_metrics/rte_metrics.c    | 7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

-- 
2.16.2

[dpdk-dev] [PATCH v3 1/2] rte_mbuf: fix strncpy warnings

[Stephen Hemminger]

Gcc-8 discovers issue with platform_mempool_ops.
rte_mbuf_pool_ops.c:26:3: error: ‘strncpy’ output truncated before
  terminating nul copying as many bytes from a string as its length
  [-Werror=stringop-truncation]
  strncpy(mz->addr, ops_name,  strlen(ops_name));

Since the ops_name is already checked for size, using strncpy
here is unnecessary; just use strcpy.

Fixes: a3acc3144a76 ("mbuf: add pool ops selection functions")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_mbuf/rte_mbuf_pool_ops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf_pool_ops.c b/lib/librte_mbuf/rte_mbuf_pool_ops.c
index 48cc342002a5..a1d4699f67fe 100644
--- a/lib/librte_mbuf/rte_mbuf_pool_ops.c
+++ b/lib/librte_mbuf/rte_mbuf_pool_ops.c
@@ -23,7 +23,7 @@ rte_mbuf_set_platform_mempool_ops(const char *ops_name)
 			RTE_MEMPOOL_OPS_NAMESIZE, SOCKET_ID_ANY, 0);
 		if (mz == NULL)
 			return -rte_errno;
-		strncpy(mz->addr, ops_name, strlen(ops_name));
+		strcpy(mz->addr, ops_name);
 		return 0;
 	} else if (strcmp(mz->addr, ops_name) == 0) {
 		return 0;
@@ -62,7 +62,7 @@ rte_mbuf_set_user_mempool_ops(const char *ops_name)
 			return -rte_errno;
 	}
 
-	strncpy(mz->addr, ops_name, strlen(ops_name));
+	strcpy(mz->addr, ops_name);
 	return 0;
 
 }
-- 
2.16.2

Re: [dpdk-dev] [PATCH v3 1/2] rte_mbuf: fix strncpy warnings

[Thomas Monjalon]

29/03/2018 19:05, Stephen Hemminger:
> Gcc-8 discovers issue with platform_mempool_ops.
> rte_mbuf_pool_ops.c:26:3: error: ‘strncpy’ output truncated before
>   terminating nul copying as many bytes from a string as its length
>   [-Werror=stringop-truncation]
>   strncpy(mz->addr, ops_name,  strlen(ops_name));
> 
> Since the ops_name is already checked for size, using strncpy
> here is unnecessary; just use strcpy.
> 
> Fixes: a3acc3144a76 ("mbuf: add pool ops selection functions")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied, thanks

[dpdk-dev] [PATCH v3 2/2] rte_metrics: fix strncpy truncation warning

[Stephen Hemminger]

Fixes:
librte_metrics/rte_metrics.c:218:4: error: ‘strncpy’ specified
 bound 64 equals destination size [-Werror=stringop-truncation]
    strncpy(names[idx_name].name,
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     stats->metadata[idx_name].name,
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     RTE_METRICS_MAX_NAME_LEN);
     ~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_metrics/rte_metrics.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 556ae1ba8b4d..e0f803ef0880 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -214,10 +214,13 @@ rte_metrics_get_names(struct rte_metric_name *names,
 			rte_spinlock_unlock(&stats->lock);
 			return return_value;
 		}
-		for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
+
+		for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++) {
 			strncpy(names[idx_name].name,
 				stats->metadata[idx_name].name,
-				RTE_METRICS_MAX_NAME_LEN);
+				RTE_METRICS_MAX_NAME_LEN - 1);
+			names[idx_name].name[RTE_METRICS_MAX_NAME_LEN - 1] = '\0';
+		}
 	}
 	return_value = stats->cnt_stats;
 	rte_spinlock_unlock(&stats->lock);
-- 
2.16.2

Re: [dpdk-dev] [PATCH v3 2/2] rte_metrics: fix strncpy truncation warning

[Thomas Monjalon]

29/03/2018 19:05, Stephen Hemminger:
> Fixes:
> librte_metrics/rte_metrics.c:218:4: error: ‘strncpy’ specified
>  bound 64 equals destination size [-Werror=stringop-truncation]
>     strncpy(names[idx_name].name,
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      stats->metadata[idx_name].name,
>      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      RTE_METRICS_MAX_NAME_LEN);
>      ~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

This one is fixed with strlcpy now.

Re: [dpdk-dev] [PATCH v3 0/2] gcc-8 build fixes

[Ferruh Yigit]

On 3/29/2018 6:05 PM, Stephen Hemminger wrote:
> This fixes some of the obvious warnings found building DPDK
> with gcc-8. There still are some deeper issues in the rte_hash_table
> code; leave the fix for that up to the maintainer.
> 
> Stephen Hemminger (2):
>   rte_mbuf: fix strncpy warnings
>   rte_metrics: fix strncpy truncation warning
> 
> v3
>   missing SOB on 1st patch
> 
> v2
>   fix issues with wrong length in mbuf pool_ops
>   don't need memset in metrics names
> 
> Stephen Hemminger (2):
>   rte_mbuf: fix strncpy warnings
>   rte_metrics: fix strncpy truncation warning

I tried with gcc-8 [1] and getting a few more build errors similar to these
ones. Are these two files only build error you get?


[1]
gcc (GCC) 8.0.1 20180401 (experimental)

Re: [dpdk-dev] [PATCH v3 0/2] gcc-8 build fixes

[Stephen Hemminger]

On Tue, 3 Apr 2018 10:23:43 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 3/29/2018 6:05 PM, Stephen Hemminger wrote:
> > This fixes some of the obvious warnings found building DPDK
> > with gcc-8. There still are some deeper issues in the rte_hash_table
> > code; leave the fix for that up to the maintainer.
> > 
> > Stephen Hemminger (2):
> >   rte_mbuf: fix strncpy warnings
> >   rte_metrics: fix strncpy truncation warning
> > 
> > v3
> >   missing SOB on 1st patch
> > 
> > v2
> >   fix issues with wrong length in mbuf pool_ops
> >   don't need memset in metrics names
> > 
> > Stephen Hemminger (2):
> >   rte_mbuf: fix strncpy warnings
> >   rte_metrics: fix strncpy truncation warning  
> 
> I tried with gcc-8 [1] and getting a few more build errors similar to these
> ones. Are these two files only build error you get?
> 
> 
> [1]
> gcc (GCC) 8.0.1 20180401 (experimental)
> 

This fixes the easy ones. The harder one is in cuckoo hash.

  CC rte_table_hash_cuckoo.o
lib/librte_table/rte_table_hash_cuckoo.c: In function ‘rte_table_hash_cuckoo_create’:
lib/librte_table/rte_table_hash_cuckoo.c:110:16: error: cast between incompatible function types from ‘rte_table_hash_op_hash’ {aka ‘long unsigned int (*)(void *, void *, unsigned int,  long unsigned int)’} to ‘uint32_t (*)(const void *, uint32_t,  uint32_t)’ {aka ‘unsigned int (*)(const void *, unsigned int,  unsigned int)’} [-Werror=cast-function-type]
   .hash_func = (rte_hash_function)(p->f_hash),
                ^
cc1: all warnings being treated as errors

Not sure what the right way to fix this one is. Hash table should not be defining
its own special hash function prototype. Changing to a common definition is
non-trivial and breaks ABI.  Casting seems wrong, error prone,
and a bad precedent in this case.