[dpdk-dev] [PATCH 00/10] Enable DPDK core build with gcc 7

[dpdk-dev] [PATCH 00/10] Enable DPDK core build with gcc 7

[Bruce Richardson]

Since GCC 7 is now yesterday's news :-), I figured we should try and make
DPDK compile successfully with that new version. The main difficulties are
because of new warnings being triggered.

* New warnings for fall-though in case statements
* New warnings for possible truncation using snprintf
* New warnings about uninitialized values.

This set of patches fixes or works around those new issues, fixing a couple of
bugs in the process, so that DPDK can compile using gcc 7.

NOTE: this set only covers the basic DPDK libs, apps and tests. It does not
cover the example apps, which still have issues.

Bruce Richardson (10):
  mk: adjust gcc flags for new gcc 7 warnings
  drivers/net: disable new gcc 7 warnings for base code
  net/bnx2x: fix warnings about switch fall-through
  net/ixgbe: fix gcc 7 warning for switch fallthrough
  net/vmxnet3: fix compile error with gcc 7
  lib: fix gcc 7 warnings for switch fall-through
  net: fix missing break inside conditional compile block
  app/testpmd: document explicit switch fall-through
  test/test: fix missing break in switch
  test/test: fix gcc 7 compiler error

 app/test-pmd/cmdline.c                 | 2 ++
 drivers/net/bnx2x/ecore_sp.c           | 2 +-
 drivers/net/bnx2x/elink.c              | 3 +++
 drivers/net/e1000/Makefile             | 3 +++
 drivers/net/fm10k/Makefile             | 3 +++
 drivers/net/ixgbe/Makefile             | 3 +++
 drivers/net/ixgbe/ixgbe_fdir.c         | 2 ++
 drivers/net/qede/Makefile              | 3 +++
 drivers/net/vmxnet3/vmxnet3_ethdev.c   | 2 +-
 lib/librte_cmdline/cmdline_parse_num.c | 4 ++--
 lib/librte_hash/rte_hash_crc.h         | 6 ++++++
 lib/librte_mbuf/rte_mbuf.h             | 4 ++++
 lib/librte_net/rte_net_crc.c           | 2 +-
 mk/toolchain/gcc/rte.vars.mk           | 7 +++++++
 test/test/test_cmdline_num.c           | 1 +
 test/test/test_eventdev_sw.c           | 2 +-
 16 files changed, 43 insertions(+), 6 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH 01/10] mk: adjust gcc flags for new gcc 7 warnings

[Bruce Richardson]

There are two new warnings in GCC 7 that cause problems in the DPDK
compile.

1. GCC now warns if you have a switch fall-through without a suitable
comment indicating that it was intentional. The compiler supports a number
of levels of warning which are triggered depending on the type of message
used, with level 3 being the default. To accept a wider range of possible
fall-through messages, we adjust this down to level 2.

2. GCC also warns about an snprintf where there may be truncation and the
return value is not checked. Given that we often use snprintf in DPDK in
place of strncpy, and in many cases where truncation is not a problem, we
can just disable this particular warning.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 mk/toolchain/gcc/rte.vars.mk | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index 5caa600..3834e00 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -99,5 +99,12 @@ ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
 WERROR_FLAGS += -Wno-uninitialized
 endif
 
+ifeq ($(shell test $(GCC_VERSION) -gt 70 && echo 1), 1)
+# Tell GCC only to error for switch fallthroughs without a suitable comment
+WERROR_FLAGS += -Wimplicit-fallthrough=2
+# Ignore errors for snprintf truncation
+WERROR_FLAGS += -Wno-format-truncation
+endif
+
 export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
 export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
-- 
2.9.3

[dpdk-dev] [PATCH 02/10] drivers/net: disable new gcc 7 warnings for base code

[Bruce Richardson]

For base code in drivers shared with other projects, disable the new
warnings from gcc 7 about unlabelled fall-through in switch statements.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/e1000/Makefile | 3 +++
 drivers/net/fm10k/Makefile | 3 +++
 drivers/net/ixgbe/Makefile | 3 +++
 drivers/net/qede/Makefile  | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index a32fabe..b5592d6 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -57,6 +57,9 @@ CFLAGS_BASE_DRIVER += -Wno-unused-variable
 ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
 ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
 CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
+ifeq ($(shell test $(GCC_VERSION) -ge 70 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-implicit-fallthrough
+endif
 endif
 endif
 endif
diff --git a/drivers/net/fm10k/Makefile b/drivers/net/fm10k/Makefile
index a8e8136..e0024f0 100644
--- a/drivers/net/fm10k/Makefile
+++ b/drivers/net/fm10k/Makefile
@@ -71,6 +71,9 @@ CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
 ifeq ($(shell test $(GCC_VERSION) -ge 44 && echo 1), 1)
 CFLAGS     += -Wno-deprecated
 CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
+ifeq ($(shell test $(GCC_VERSION) -ge 70 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-implicit-fallthrough
+endif
 endif
 endif
 
diff --git a/drivers/net/ixgbe/Makefile b/drivers/net/ixgbe/Makefile
index 0a6b7f2..5529d81 100644
--- a/drivers/net/ixgbe/Makefile
+++ b/drivers/net/ixgbe/Makefile
@@ -76,6 +76,9 @@ endif
 
 ifeq ($(shell test $(GCC_VERSION) -ge 50 && echo 1), 1)
 CFLAGS_ixgbe_common.o += -Wno-logical-not-parentheses
+ifeq ($(shell test $(GCC_VERSION) -ge 70 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-implicit-fallthrough
+endif
 endif
 
 endif
diff --git a/drivers/net/qede/Makefile b/drivers/net/qede/Makefile
index 8acef00..3323914 100644
--- a/drivers/net/qede/Makefile
+++ b/drivers/net/qede/Makefile
@@ -56,6 +56,9 @@ endif
 CFLAGS_BASE_DRIVER += -Wno-strict-prototypes
 ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
 CFLAGS_BASE_DRIVER += -Wno-shift-negative-value
+ifeq ($(shell test $(GCC_VERSION) -ge 70 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-implicit-fallthrough
+endif
 endif
 else ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
 CFLAGS_BASE_DRIVER += -Wno-format-extra-args
-- 
2.9.3

[dpdk-dev] [PATCH 03/10] net/bnx2x: fix warnings about switch fall-through

[Bruce Richardson]

Add in a comment for each switch fall-through indicating that it is
intentional. This will fix compiler warnings with GCC 7.

Fixes: b5bf7719221d ("bnx2x: driver support routines")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/bnx2x/ecore_sp.c | 2 +-
 drivers/net/bnx2x/elink.c    | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnx2x/ecore_sp.c b/drivers/net/bnx2x/ecore_sp.c
index e6fecd8..22f2dc9 100644
--- a/drivers/net/bnx2x/ecore_sp.c
+++ b/drivers/net/bnx2x/ecore_sp.c
@@ -2725,7 +2725,7 @@ static int ecore_mcast_validate_e2(__rte_unused struct bnx2x_softc *sc,
 		/* DEL command deletes all currently configured MACs */
 	case ECORE_MCAST_CMD_DEL:
 		o->set_registry_size(o, 0);
-		/* Don't break */
+		/* fall-through */
 
 		/* RESTORE command will restore the entire multicast configuration */
 	case ECORE_MCAST_CMD_RESTORE:
diff --git a/drivers/net/bnx2x/elink.c b/drivers/net/bnx2x/elink.c
index 5329396..9ffa7dc 100644
--- a/drivers/net/bnx2x/elink.c
+++ b/drivers/net/bnx2x/elink.c
@@ -5898,6 +5898,7 @@ elink_status_t elink_set_led(struct elink_params *params,
 		 */
 		if (!vars->link_up)
 			break;
+		/* fall-through */
 	case ELINK_LED_MODE_ON:
 		if (((params->phy[ELINK_EXT_PHY1].type ==
 		      PORT_HW_CFG_XGXS_EXT_PHY_TYPE_BNX2X8727) ||
@@ -11534,11 +11535,13 @@ static void elink_phy_def_cfg(struct elink_params *params,
 	switch (link_config & PORT_FEATURE_LINK_SPEED_MASK) {
 	case PORT_FEATURE_LINK_SPEED_10M_HALF:
 		phy->req_duplex = DUPLEX_HALF;
+		/* fall-through */
 	case PORT_FEATURE_LINK_SPEED_10M_FULL:
 		phy->req_line_speed = ELINK_SPEED_10;
 		break;
 	case PORT_FEATURE_LINK_SPEED_100M_HALF:
 		phy->req_duplex = DUPLEX_HALF;
+		/* fall-through */
 	case PORT_FEATURE_LINK_SPEED_100M_FULL:
 		phy->req_line_speed = ELINK_SPEED_100;
 		break;
-- 
2.9.3

[dpdk-dev] [PATCH 04/10] net/ixgbe: fix gcc 7 warning for switch fallthrough

[Bruce Richardson]

Add a comment documenting explicitly that we are falling through the case
statements to the next one.

Fixes: f9072f8b90bb ("ixgbe: migrate flow director filtering to new API")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/ixgbe/ixgbe_fdir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_fdir.c b/drivers/net/ixgbe/ixgbe_fdir.c
index d6e48e9..7f6c7b5 100644
--- a/drivers/net/ixgbe/ixgbe_fdir.c
+++ b/drivers/net/ixgbe/ixgbe_fdir.c
@@ -730,6 +730,7 @@ ixgbe_fdir_filter_to_atr_input(const struct rte_eth_fdir_filter *fdir_filter,
 			fdir_filter->input.flow.udp4_flow.src_port;
 		input->formatted.dst_port =
 			fdir_filter->input.flow.udp4_flow.dst_port;
+		/* fall-through */
 	/*for SCTP flow type, port and verify_tag are meaningless in ixgbe.*/
 	case RTE_ETH_FLOW_NONFRAG_IPV4_SCTP:
 	case RTE_ETH_FLOW_NONFRAG_IPV4_OTHER:
@@ -745,6 +746,7 @@ ixgbe_fdir_filter_to_atr_input(const struct rte_eth_fdir_filter *fdir_filter,
 			fdir_filter->input.flow.udp6_flow.src_port;
 		input->formatted.dst_port =
 			fdir_filter->input.flow.udp6_flow.dst_port;
+		/* fall-through */
 	/*for SCTP flow type, port and verify_tag are meaningless in ixgbe.*/
 	case RTE_ETH_FLOW_NONFRAG_IPV6_SCTP:
 	case RTE_ETH_FLOW_NONFRAG_IPV6_OTHER:
-- 
2.9.3

[dpdk-dev] [PATCH 05/10] net/vmxnet3: fix compile error with gcc 7

[Bruce Richardson]

GCC 7 flags a value as uninitialized before used. While it's a false
positive, there is little harm in providing an initial value for the
variable.

Fixes: bb1d14b87fc3 ("vmxnet3: fix link state handling")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 0e8eb75..1cd72c1 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -935,7 +935,7 @@ vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 			__rte_unused int wait_to_complete)
 {
 	struct vmxnet3_hw *hw = dev->data->dev_private;
-	struct rte_eth_link old, link;
+	struct rte_eth_link old = {0}, link;
 	uint32_t ret;
 
 	/* Link status doesn't change for stopped dev */
-- 
2.9.3

[dpdk-dev] [PATCH 06/10] lib: fix gcc 7 warnings for switch fall-through

[Bruce Richardson]

With GCC 7 we need to explicitly document when we are falling through from
one switch case to another.

Fixes: af75078fece3 ("first public release")
Fixes: 8bae1da2afe0 ("hash: fallback to software CRC32 implementation")
Fixes: 9ec201f5d6e7 ("mbuf: provide bulk allocation")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_cmdline/cmdline_parse_num.c | 4 ++--
 lib/librte_hash/rte_hash_crc.h         | 6 ++++++
 lib/librte_mbuf/rte_mbuf.h             | 4 ++++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/librte_cmdline/cmdline_parse_num.c b/lib/librte_cmdline/cmdline_parse_num.c
index b0f9a35..e507ec4 100644
--- a/lib/librte_cmdline/cmdline_parse_num.c
+++ b/lib/librte_cmdline/cmdline_parse_num.c
@@ -250,7 +250,7 @@ cmdline_parse_num(cmdline_parse_token_hdr_t *tk, const char *srcbuf, void *res,
 
 		case HEX:
 			st = HEX_OK;
-			/* no break */
+			/* fall-through no break */
 		case HEX_OK:
 			if (c >= '0' && c <= '9') {
 				if (add_to_res(c - '0', &res1, 16) < 0)
@@ -282,7 +282,7 @@ cmdline_parse_num(cmdline_parse_token_hdr_t *tk, const char *srcbuf, void *res,
 
 		case BIN:
 			st = BIN_OK;
-			/* no break */
+			/* fall-through */
 		case BIN_OK:
 			if (c >= '0' && c <= '1') {
 				if (add_to_res(c - '0', &res1, 2) < 0)
diff --git a/lib/librte_hash/rte_hash_crc.h b/lib/librte_hash/rte_hash_crc.h
index 63e74aa..0f485b8 100644
--- a/lib/librte_hash/rte_hash_crc.h
+++ b/lib/librte_hash/rte_hash_crc.h
@@ -476,9 +476,15 @@ rte_hash_crc_set_alg(uint8_t alg)
 	case CRC32_SSE42_x64:
 		if (! rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T))
 			alg = CRC32_SSE42;
+#if __GNUC__ >= 7
+		__attribute__ ((fallthrough));
+#endif
 	case CRC32_SSE42:
 		if (! rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_2))
 			alg = CRC32_SW;
+#if __GNUC__ >= 7
+		__attribute__ ((fallthrough));
+#endif
 #endif
 	case CRC32_SW:
 		crc32_alg = alg;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 466ec00..9097f18 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1156,21 +1156,25 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
 			rte_mbuf_refcnt_set(mbufs[idx], 1);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
+			/* fall-through */
 	case 3:
 			RTE_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0);
 			rte_mbuf_refcnt_set(mbufs[idx], 1);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
+			/* fall-through */
 	case 2:
 			RTE_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0);
 			rte_mbuf_refcnt_set(mbufs[idx], 1);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
+			/* fall-through */
 	case 1:
 			RTE_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0);
 			rte_mbuf_refcnt_set(mbufs[idx], 1);
 			rte_pktmbuf_reset(mbufs[idx]);
 			idx++;
+			/* fall-through */
 		}
 	}
 	return 0;
-- 
2.9.3

[dpdk-dev] [PATCH 07/10] net: fix missing break inside conditional compile block

[Bruce Richardson]

The #ifdef only had the break in the else leg rather than in the first leg,
leading to the value set their being overridden on fall-through.

Fixes: 986ff526fb84 ("net: add CRC computation API")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_net/rte_net_crc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_net_crc.c b/lib/librte_net/rte_net_crc.c
index e8326fe..9d1ee63 100644
--- a/lib/librte_net/rte_net_crc.c
+++ b/lib/librte_net/rte_net_crc.c
@@ -167,8 +167,8 @@ rte_net_crc_set_alg(enum rte_net_crc_alg alg)
 		handlers = handlers_sse42;
 #else
 		alg = RTE_NET_CRC_SCALAR;
-		break;
 #endif
+		break;
 	case RTE_NET_CRC_SCALAR:
 	default:
 		handlers = handlers_scalar;
-- 
2.9.3

[dpdk-dev] [PATCH 08/10] app/testpmd: document explicit switch fall-through

[Bruce Richardson]

This fixes compiler warnings with GCC 7.

Fixes: 28d62131a1b1 ("app/testpmd: extend flow director input set commands")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test-pmd/cmdline.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 06c1ce2..0afac68 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8741,6 +8741,7 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 	case RTE_ETH_FLOW_FRAG_IPV4:
 	case RTE_ETH_FLOW_NONFRAG_IPV4_OTHER:
 		entry.input.flow.ip4_flow.proto = res->proto_value;
+		/* fall-through */
 	case RTE_ETH_FLOW_NONFRAG_IPV4_UDP:
 	case RTE_ETH_FLOW_NONFRAG_IPV4_TCP:
 		IPV4_ADDR_TO_UINT(res->ip_dst,
@@ -8773,6 +8774,7 @@ cmd_flow_director_filter_parsed(void *parsed_result,
 	case RTE_ETH_FLOW_FRAG_IPV6:
 	case RTE_ETH_FLOW_NONFRAG_IPV6_OTHER:
 		entry.input.flow.ipv6_flow.proto = res->proto_value;
+		/* fall-through */
 	case RTE_ETH_FLOW_NONFRAG_IPV6_UDP:
 	case RTE_ETH_FLOW_NONFRAG_IPV6_TCP:
 		IPV6_ADDR_TO_ARRAY(res->ip_dst,
-- 
2.9.3

[dpdk-dev] [PATCH 09/10] test/test: fix missing break in switch

[Bruce Richardson]

Issue flagged by GCC 7 as a switch fall-through.

Fixes: dbb860e03eb1 ("cmdline: tests")

CC: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_cmdline_num.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/test/test_cmdline_num.c b/test/test/test_cmdline_num.c
index 04263d3..e8f60cf 100644
--- a/test/test/test_cmdline_num.c
+++ b/test/test/test_cmdline_num.c
@@ -315,6 +315,7 @@ can_parse_signed(int64_t expected_result, enum cmdline_numtype type)
 	case UINT64:
 		if (expected_result < 0)
 			return 0;
+		break;
 	case INT8:
 		if (expected_result > INT8_MAX || expected_result < INT8_MIN)
 			return 0;
-- 
2.9.3

[dpdk-dev] [PATCH 10/10] test/test: fix gcc 7 compiler error

[Bruce Richardson]

GCC flags an uninitialized value, so provide an initializer.

Fixes: 3a17ff401f1e ("test/eventdev: add basic SW tests")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_eventdev_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test/test_eventdev_sw.c b/test/test/test_eventdev_sw.c
index fd6447e..3abeaef 100644
--- a/test/test/test_eventdev_sw.c
+++ b/test/test/test_eventdev_sw.c
@@ -2526,7 +2526,7 @@ parallel_basic(struct test *t, int check_order)
 	int i;
 	uint32_t deq_pkts, j;
 	struct rte_mbuf *mbufs[3];
-	struct rte_mbuf *mbufs_out[3];
+	struct rte_mbuf *mbufs_out[3] = {0};
 	const uint32_t MAGIC_SEQN = 1234;
 
 	/* Create instance with 4 ports */
-- 
2.9.3

Re: [dpdk-dev] [PATCH 01/10] mk: adjust gcc flags for new gcc 7 warnings

[Stephen Hemminger]

On Thu,  4 May 2017 16:38:13 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> There are two new warnings in GCC 7 that cause problems in the DPDK
> compile.
> 
> 1. GCC now warns if you have a switch fall-through without a suitable
> comment indicating that it was intentional. The compiler supports a number
> of levels of warning which are triggered depending on the type of message
> used, with level 3 being the default. To accept a wider range of possible
> fall-through messages, we adjust this down to level 2.
> 
> 2. GCC also warns about an snprintf where there may be truncation and the
> return value is not checked. Given that we often use snprintf in DPDK in
> place of strncpy, and in many cases where truncation is not a problem, we
> can just disable this particular warning.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  mk/toolchain/gcc/rte.vars.mk | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
> index 5caa600..3834e00 100644
> --- a/mk/toolchain/gcc/rte.vars.mk
> +++ b/mk/toolchain/gcc/rte.vars.mk
> @@ -99,5 +99,12 @@ ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
>  WERROR_FLAGS += -Wno-uninitialized
>  endif
>  
> +ifeq ($(shell test $(GCC_VERSION) -gt 70 && echo 1), 1)
> +# Tell GCC only to error for switch fallthroughs without a suitable comment
> +WERROR_FLAGS += -Wimplicit-fallthrough=2
> +# Ignore errors for snprintf truncation
> +WERROR_FLAGS += -Wno-format-truncation
> +endif
> +
>  export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
>  export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS

Please fix the code not neuter  warnings

Re: [dpdk-dev] [PATCH 05/10] net/vmxnet3: fix compile error with gcc 7

[Stephen Hemminger]

On Thu,  4 May 2017 16:38:17 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 0e8eb75..1cd72c1 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -935,7 +935,7 @@ vmxnet3_dev_link_update(struct rte_eth_dev *dev,
>  			__rte_unused int wait_to_complete)
>  {
>  	struct vmxnet3_hw *hw = dev->data->dev_private;
> -	struct rte_eth_link old, link;
> +	struct rte_eth_link old = {0}, link;

Please add space around the initializer.
ie. 
	struct rte_eth_link old = { 0 }, link;

Re: [dpdk-dev] [PATCH 04/10] net/ixgbe: fix gcc 7 warning for switch fallthrough

[Lu, Wenzhuo]

Hi,


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Thursday, May 4, 2017 11:38 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce
> Subject: [dpdk-dev] [PATCH 04/10] net/ixgbe: fix gcc 7 warning for switch
> fallthrough
> 
> Add a comment documenting explicitly that we are falling through the case
> statements to the next one.
> 
> Fixes: f9072f8b90bb ("ixgbe: migrate flow director filtering to new API")
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [PATCH 07/10] net: fix missing break inside conditional compile block

[Singh, Jasvinder]

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
Sent: Thursday, May 4, 2017 4:38 PM
To: dev@dpdk.org
Cc: Richardson, Bruce <bruce.richardson@intel.com>
Subject: [dpdk-dev] [PATCH 07/10] net: fix missing break inside conditional compile block

The #ifdef only had the break in the else leg rather than in the first leg, leading to the value set their being overridden on fall-through.

Fixes: 986ff526fb84 ("net: add CRC computation API")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Jasvinder Singh <jasvinder.singh@intel.com>

Re: [dpdk-dev] [PATCH 01/10] mk: adjust gcc flags for new gcc 7 warnings

[Bruce Richardson]

On Thu, May 04, 2017 at 09:38:08AM -0700, Stephen Hemminger wrote:
> On Thu,  4 May 2017 16:38:13 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > There are two new warnings in GCC 7 that cause problems in the DPDK
> > compile.
> > 
> > 1. GCC now warns if you have a switch fall-through without a suitable
> > comment indicating that it was intentional. The compiler supports a number
> > of levels of warning which are triggered depending on the type of message
> > used, with level 3 being the default. To accept a wider range of possible
> > fall-through messages, we adjust this down to level 2.
> > 
> > 2. GCC also warns about an snprintf where there may be truncation and the
> > return value is not checked. Given that we often use snprintf in DPDK in
> > place of strncpy, and in many cases where truncation is not a problem, we
> > can just disable this particular warning.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  mk/toolchain/gcc/rte.vars.mk | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
> > index 5caa600..3834e00 100644
> > --- a/mk/toolchain/gcc/rte.vars.mk
> > +++ b/mk/toolchain/gcc/rte.vars.mk
> > @@ -99,5 +99,12 @@ ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
> >  WERROR_FLAGS += -Wno-uninitialized
> >  endif
> >  
> > +ifeq ($(shell test $(GCC_VERSION) -gt 70 && echo 1), 1)
> > +# Tell GCC only to error for switch fallthroughs without a suitable comment
> > +WERROR_FLAGS += -Wimplicit-fallthrough=2
> > +# Ignore errors for snprintf truncation
> > +WERROR_FLAGS += -Wno-format-truncation
> > +endif
> > +
> >  export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
> >  export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
> 
> Please fix the code not neuter  warnings

As much as is possible I agree. However, for these two warnings:

1. the implicit fallthrough warning is not disabled, it's just adjusted
to allow a wider range of comments for fall-through to be accepted. The
rest of the set implements fixes for a number of these warnings in the
code.

2. for the format truncation warning, ideally, yes we should fix the
code, except that I don't believe this is feasible in the short term,
and I also don't believe it is desirable. We extensively use snprintf
because it has sane/safe truncation, and in many cases we don't care if
it is being truncated. Therefore disabling the warning seems the best
approach to me. Furthermore, if we want 17.05 to compile with GCC 7,
this is the best option within that timeframe.

Regards,
/Bruce

Re: [dpdk-dev] [PATCH 01/10] mk: adjust gcc flags for new gcc 7 warnings

[Thomas Monjalon]

In this series, there are some fixes for fall-through comments,
missing break and missing initializers.
I think there is no discussion about accepting them in 17.05.
The last item to discuss it the new snprintf warning:

05/05/2017 11:42, Bruce Richardson:
> On Thu, May 04, 2017 at 09:38:08AM -0700, Stephen Hemminger wrote:
> > On Thu,  4 May 2017 16:38:13 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> > > 2. GCC also warns about an snprintf where there may be truncation and the
> > > return value is not checked. Given that we often use snprintf in DPDK in
> > > place of strncpy, and in many cases where truncation is not a problem, we
> > > can just disable this particular warning.
[...]
> > > --- a/mk/toolchain/gcc/rte.vars.mk
> > > +++ b/mk/toolchain/gcc/rte.vars.mk
> > > +# Ignore errors for snprintf truncation
> > > +WERROR_FLAGS += -Wno-format-truncation
[...]
> 2. for the format truncation warning, ideally, yes we should fix the
> code, except that I don't believe this is feasible in the short term,
> and I also don't believe it is desirable. We extensively use snprintf
> because it has sane/safe truncation, and in many cases we don't care if
> it is being truncated. Therefore disabling the warning seems the best
> approach to me. Furthermore, if we want 17.05 to compile with GCC 7,
> this is the best option within that timeframe.

We could imagine an explicit ignore of the return code.
However, do we really want this new coding rule for every snprintf?
It is a common call in DPDK:
	git grep '\<snprintf\>' | wc -l
	774
And probably almost never checked:
	git grep '^[[:space:]]*\<snprintf\>' | wc -l
	660

I suggest to disable this new warning in GCC 7.
Any opinions?

Re: [dpdk-dev] [PATCH 01/10] mk: adjust gcc flags for new gcc 7 warnings

[Bruce Richardson]

On Fri, May 05, 2017 at 12:02:44PM +0200, Thomas Monjalon wrote:
> In this series, there are some fixes for fall-through comments,
> missing break and missing initializers.
> I think there is no discussion about accepting them in 17.05.
> The last item to discuss it the new snprintf warning:
> 
> 05/05/2017 11:42, Bruce Richardson:
> > On Thu, May 04, 2017 at 09:38:08AM -0700, Stephen Hemminger wrote:
> > > On Thu,  4 May 2017 16:38:13 +0100
> > > Bruce Richardson <bruce.richardson@intel.com> wrote:
> > > > 2. GCC also warns about an snprintf where there may be truncation and the
> > > > return value is not checked. Given that we often use snprintf in DPDK in
> > > > place of strncpy, and in many cases where truncation is not a problem, we
> > > > can just disable this particular warning.
> [...]
> > > > --- a/mk/toolchain/gcc/rte.vars.mk
> > > > +++ b/mk/toolchain/gcc/rte.vars.mk
> > > > +# Ignore errors for snprintf truncation
> > > > +WERROR_FLAGS += -Wno-format-truncation
> [...]
> > 2. for the format truncation warning, ideally, yes we should fix the
> > code, except that I don't believe this is feasible in the short term,
> > and I also don't believe it is desirable. We extensively use snprintf
> > because it has sane/safe truncation, and in many cases we don't care if
> > it is being truncated. Therefore disabling the warning seems the best
> > approach to me. Furthermore, if we want 17.05 to compile with GCC 7,
> > this is the best option within that timeframe.
> 
> We could imagine an explicit ignore of the return code.
> However, do we really want this new coding rule for every snprintf?
> It is a common call in DPDK:
> 	git grep '\<snprintf\>' | wc -l
> 	774
> And probably almost never checked:
> 	git grep '^[[:space:]]*\<snprintf\>' | wc -l
> 	660
> 
> I suggest to disable this new warning in GCC 7.
> Any opinions?

I'd suggest that even fewer than that are actually recorded, let alone
checked:

git grep '= *snprintf\>' | wc -l
89

So I'm (obviously) +1 for dropping this warning check.

/Bruce

Re: [dpdk-dev] [PATCH 01/10] mk: adjust gcc flags for new gcc 7 warnings

[Van Haaren, Harry]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Friday, May 5, 2017 11:03 AM
> To: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: Re: [dpdk-dev] [PATCH 01/10] mk: adjust gcc flags for new gcc 7 warnings
> 
> In this series, there are some fixes for fall-through comments,
> missing break and missing initializers.
> I think there is no discussion about accepting them in 17.05.
> The last item to discuss it the new snprintf warning:
> 
> 05/05/2017 11:42, Bruce Richardson:
> > On Thu, May 04, 2017 at 09:38:08AM -0700, Stephen Hemminger wrote:
> > > On Thu,  4 May 2017 16:38:13 +0100
> > > Bruce Richardson <bruce.richardson@intel.com> wrote:
> > > > 2. GCC also warns about an snprintf where there may be truncation and the
> > > > return value is not checked. Given that we often use snprintf in DPDK in
> > > > place of strncpy, and in many cases where truncation is not a problem, we
> > > > can just disable this particular warning.
> [...]
> > > > --- a/mk/toolchain/gcc/rte.vars.mk
> > > > +++ b/mk/toolchain/gcc/rte.vars.mk
> > > > +# Ignore errors for snprintf truncation
> > > > +WERROR_FLAGS += -Wno-format-truncation
> [...]
> > 2. for the format truncation warning, ideally, yes we should fix the
> > code, except that I don't believe this is feasible in the short term,
> > and I also don't believe it is desirable. We extensively use snprintf
> > because it has sane/safe truncation, and in many cases we don't care if
> > it is being truncated. Therefore disabling the warning seems the best
> > approach to me. Furthermore, if we want 17.05 to compile with GCC 7,
> > this is the best option within that timeframe.
> 
> We could imagine an explicit ignore of the return code.
> However, do we really want this new coding rule for every snprintf?
> It is a common call in DPDK:
> 	git grep '\<snprintf\>' | wc -l
> 	774
> And probably almost never checked:
> 	git grep '^[[:space:]]*\<snprintf\>' | wc -l
> 	660
> 
> I suggest to disable this new warning in GCC 7.
> Any opinions?

+1 to disable, it seems a pragmatic solution in this case.

Re: [dpdk-dev] [PATCH 00/10] Enable DPDK core build with gcc 7

[Thomas Monjalon]

04/05/2017 17:38, Bruce Richardson:
> Since GCC 7 is now yesterday's news :-), I figured we should try and make
> DPDK compile successfully with that new version. The main difficulties are
> because of new warnings being triggered.
> 
> * New warnings for fall-though in case statements
> * New warnings for possible truncation using snprintf
> * New warnings about uninitialized values.
> 
> This set of patches fixes or works around those new issues, fixing a couple of
> bugs in the process, so that DPDK can compile using gcc 7.
> 
> NOTE: this set only covers the basic DPDK libs, apps and tests. It does not
> cover the example apps, which still have issues.
> 
> Bruce Richardson (10):
>   mk: adjust gcc flags for new gcc 7 warnings
>   drivers/net: disable new gcc 7 warnings for base code
>   net/bnx2x: fix warnings about switch fall-through
>   net/ixgbe: fix gcc 7 warning for switch fallthrough
>   net/vmxnet3: fix compile error with gcc 7
>   lib: fix gcc 7 warnings for switch fall-through
>   net: fix missing break inside conditional compile block
>   app/testpmd: document explicit switch fall-through
>   test/test: fix missing break in switch
>   test/test: fix gcc 7 compiler error

Applied (with small space fixes), thanks