[dpdk-dev] [PATCH 1/2] e1000:fix gcc test for clang builds

[dpdk-dev] [PATCH 1/2] e1000:fix gcc test for clang builds

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/e1000/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index 5b801f5..1305163 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -48,7 +48,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
 # CFLAGS for icc
 #
 CFLAGS_BASE_DRIVER = -wd177 -wd181 -wd188 -wd869 -wd2259
-else
+else ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
 #
 # CFLAGS for gcc
 #
@@ -57,6 +57,9 @@ CFLAGS_BASE_DRIVER += -Wno-unused-variable
 ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
 CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
 endif
+else
+CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
+CFLAGS_BASE_DRIVER += -Wno-unused-variable
 endif
 
 #
-- 
2.8.0.GIT

[dpdk-dev] [PATCH 2/2] fix building with clang-3.8.0 compiler

[Keith Wiles]

Latest clang compiler 3.8.0 on latest update of Ubuntu
creates a few more warnings on -Warray-bounds and extra
() around 'if' expressions.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 app/test-pmd/Makefile                | 3 +++
 app/test/Makefile                    | 3 +++
 drivers/net/bonding/Makefile         | 4 ++++
 drivers/net/fm10k/Makefile           | 2 ++
 drivers/net/i40e/Makefile            | 2 ++
 lib/librte_cmdline/Makefile          | 6 ++++++
 lib/librte_eal/linuxapp/eal/Makefile | 7 +++++++
 7 files changed, 27 insertions(+)

diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index 2a0b5a5..14ab77c 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -59,6 +59,9 @@ SRCS-y += icmpecho.c
 SRCS-$(CONFIG_RTE_LIBRTE_IEEE1588) += ieee1588fwd.c
 
 CFLAGS_cmdline.o := -D_GNU_SOURCE
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_cmdline.o += -Wno-array-bounds
+endif
 
 # this application needs libraries first
 DEPDIRS-y += lib drivers
diff --git a/app/test/Makefile b/app/test/Makefile
index 2de8c7a..5593426 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -211,6 +211,9 @@ ifeq ($(shell test $(GCC_VERSION) -ge 44 && echo 1), 1)
 CFLAGS_test_memcpy.o += -fno-var-tracking-assignments
 CFLAGS_test_memcpy_perf.o += -fno-var-tracking-assignments
 endif
+else ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_test_errno.o += -Wno-array-bounds
+CFLAGS_test_devargs.o += -Wno-array-bounds
 endif
 
 # this application needs libraries first
diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile
index 504f2e8..ee5f44a 100644
--- a/drivers/net/bonding/Makefile
+++ b/drivers/net/bonding/Makefile
@@ -43,6 +43,10 @@ EXPORT_MAP := rte_eth_bond_version.map
 
 LIBABIVER := 1
 
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_rte_eth_bond_args.o := -Wno-array-bounds
+endif
+
 #
 # all source are stored in SRCS-y
 #
diff --git a/drivers/net/fm10k/Makefile b/drivers/net/fm10k/Makefile
index afcbd1d..d6c692d 100644
--- a/drivers/net/fm10k/Makefile
+++ b/drivers/net/fm10k/Makefile
@@ -59,6 +59,8 @@ CFLAGS_BASE_DRIVER += -Wno-strict-aliasing -Wno-format-extra-args
 CFLAGS_BASE_DRIVER += -Wno-unused-variable
 CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
 
+CFLAGS_fm10k_ethdev.o := -Wno-array-bounds
+
 else
 #
 # CFLAGS for gcc
diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
index 9ffef3f..3a9e5a1 100644
--- a/drivers/net/i40e/Makefile
+++ b/drivers/net/i40e/Makefile
@@ -60,6 +60,8 @@ CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
 CFLAGS_BASE_DRIVER += -Wno-pointer-to-int-cast
 CFLAGS_BASE_DRIVER += -Wno-format-nonliteral
 CFLAGS_BASE_DRIVER += -Wno-unused-variable
+
+CFLAGS_i40e_hmc.o += -Wno-tautological-compare
 else
 CFLAGS_BASE_DRIVER  = -Wno-sign-compare
 CFLAGS_BASE_DRIVER += -Wno-unused-value
diff --git a/lib/librte_cmdline/Makefile b/lib/librte_cmdline/Makefile
index 7d2d148..0af1d0a 100644
--- a/lib/librte_cmdline/Makefile
+++ b/lib/librte_cmdline/Makefile
@@ -40,6 +40,12 @@ EXPORT_MAP := rte_cmdline_version.map
 
 LIBABIVER := 2
 
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_cmdline_cirbuf.o:= -Wno-parentheses-equality
+CFLAGS_cmdline_rdline.o:= -Wno-parentheses-equality
+CFLAGS_cmdline_parse_string.o:= -Wno-array-bounds
+endif
+
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) := cmdline.c
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_cirbuf.c
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 30b30f3..2ff8d62 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -125,6 +125,13 @@ CFLAGS_eal_common_options.o := -D_GNU_SOURCE
 CFLAGS_eal_common_thread.o := -D_GNU_SOURCE
 CFLAGS_eal_common_lcore.o := -D_GNU_SOURCE
 
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_eal_alarm.o += -Wno-parentheses-equality
+CFLAGS_eal_common_options.o += -Wno-array-bounds
+CFLAGS_eal_pci.o += -Wno-parentheses-equality
+CFLAGS_eal_pci.o += -Wno-array-bounds
+endif
+
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
 ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
-- 
2.8.0.GIT

Re: [dpdk-dev] [PATCH 1/2] e1000:fix gcc test for clang builds

[Wiles, Keith]

On 6/26/16, 10:32 AM, "dev on behalf of Keith Wiles" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:

>Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>---
> drivers/net/e1000/Makefile | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
>index 5b801f5..1305163 100644
>--- a/drivers/net/e1000/Makefile
>+++ b/drivers/net/e1000/Makefile
>@@ -48,7 +48,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
> # CFLAGS for icc
> #
> CFLAGS_BASE_DRIVER = -wd177 -wd181 -wd188 -wd869 -wd2259
>-else
>+else ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> #
> # CFLAGS for gcc
> #
>@@ -57,6 +57,9 @@ CFLAGS_BASE_DRIVER += -Wno-unused-variable
> ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> endif
>+else
>+CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>+CFLAGS_BASE_DRIVER += -Wno-unused-variable
> endif

self-nak found another clang compiler problem.
> 
> #
>-- 
>2.8.0.GIT
>
>

Re: [dpdk-dev] [PATCH 2/2] fix building with clang-3.8.0 compiler

[Wiles, Keith]

On 6/26/16, 10:32 AM, "dev on behalf of Keith Wiles" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:

>Latest clang compiler 3.8.0 on latest update of Ubuntu
>creates a few more warnings on -Warray-bounds and extra
>() around 'if' expressions.
>
>Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>---
> app/test-pmd/Makefile                | 3 +++
> app/test/Makefile                    | 3 +++
> drivers/net/bonding/Makefile         | 4 ++++
> drivers/net/fm10k/Makefile           | 2 ++
> drivers/net/i40e/Makefile            | 2 ++
> lib/librte_cmdline/Makefile          | 6 ++++++
> lib/librte_eal/linuxapp/eal/Makefile | 7 +++++++
> 7 files changed, 27 insertions(+)
>

self-nak: found another problem

[dpdk-dev] [PATCH v2 1/2] e1000:fix gcc test for clang builds

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 drivers/net/e1000/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index 5b801f5..1305163 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -48,7 +48,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
 # CFLAGS for icc
 #
 CFLAGS_BASE_DRIVER = -wd177 -wd181 -wd188 -wd869 -wd2259
-else
+else ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
 #
 # CFLAGS for gcc
 #
@@ -57,6 +57,9 @@ CFLAGS_BASE_DRIVER += -Wno-unused-variable
 ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
 CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
 endif
+else
+CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
+CFLAGS_BASE_DRIVER += -Wno-unused-variable
 endif
 
 #
-- 
2.8.0.GIT

[dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Keith Wiles]

Latest clang compiler 3.8.0 on latest update of Ubuntu
creates a few more warnings on -Warray-bounds and extra
() around 'if' expressions.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 app/test-pmd/Makefile                | 3 +++
 app/test/Makefile                    | 3 +++
 drivers/net/bonding/Makefile         | 4 ++++
 drivers/net/fm10k/Makefile           | 2 ++
 drivers/net/i40e/Makefile            | 2 ++
 lib/librte_cmdline/Makefile          | 6 ++++++
 lib/librte_eal/linuxapp/eal/Makefile | 8 ++++++++
 7 files changed, 28 insertions(+)

diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index 2a0b5a5..14ab77c 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -59,6 +59,9 @@ SRCS-y += icmpecho.c
 SRCS-$(CONFIG_RTE_LIBRTE_IEEE1588) += ieee1588fwd.c
 
 CFLAGS_cmdline.o := -D_GNU_SOURCE
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_cmdline.o += -Wno-array-bounds
+endif
 
 # this application needs libraries first
 DEPDIRS-y += lib drivers
diff --git a/app/test/Makefile b/app/test/Makefile
index 2de8c7a..5593426 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -211,6 +211,9 @@ ifeq ($(shell test $(GCC_VERSION) -ge 44 && echo 1), 1)
 CFLAGS_test_memcpy.o += -fno-var-tracking-assignments
 CFLAGS_test_memcpy_perf.o += -fno-var-tracking-assignments
 endif
+else ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_test_errno.o += -Wno-array-bounds
+CFLAGS_test_devargs.o += -Wno-array-bounds
 endif
 
 # this application needs libraries first
diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile
index 504f2e8..ee5f44a 100644
--- a/drivers/net/bonding/Makefile
+++ b/drivers/net/bonding/Makefile
@@ -43,6 +43,10 @@ EXPORT_MAP := rte_eth_bond_version.map
 
 LIBABIVER := 1
 
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_rte_eth_bond_args.o := -Wno-array-bounds
+endif
+
 #
 # all source are stored in SRCS-y
 #
diff --git a/drivers/net/fm10k/Makefile b/drivers/net/fm10k/Makefile
index afcbd1d..d6c692d 100644
--- a/drivers/net/fm10k/Makefile
+++ b/drivers/net/fm10k/Makefile
@@ -59,6 +59,8 @@ CFLAGS_BASE_DRIVER += -Wno-strict-aliasing -Wno-format-extra-args
 CFLAGS_BASE_DRIVER += -Wno-unused-variable
 CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
 
+CFLAGS_fm10k_ethdev.o := -Wno-array-bounds
+
 else
 #
 # CFLAGS for gcc
diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
index 9ffef3f..3a9e5a1 100644
--- a/drivers/net/i40e/Makefile
+++ b/drivers/net/i40e/Makefile
@@ -60,6 +60,8 @@ CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
 CFLAGS_BASE_DRIVER += -Wno-pointer-to-int-cast
 CFLAGS_BASE_DRIVER += -Wno-format-nonliteral
 CFLAGS_BASE_DRIVER += -Wno-unused-variable
+
+CFLAGS_i40e_hmc.o += -Wno-tautological-compare
 else
 CFLAGS_BASE_DRIVER  = -Wno-sign-compare
 CFLAGS_BASE_DRIVER += -Wno-unused-value
diff --git a/lib/librte_cmdline/Makefile b/lib/librte_cmdline/Makefile
index 7d2d148..0af1d0a 100644
--- a/lib/librte_cmdline/Makefile
+++ b/lib/librte_cmdline/Makefile
@@ -40,6 +40,12 @@ EXPORT_MAP := rte_cmdline_version.map
 
 LIBABIVER := 2
 
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_cmdline_cirbuf.o:= -Wno-parentheses-equality
+CFLAGS_cmdline_rdline.o:= -Wno-parentheses-equality
+CFLAGS_cmdline_parse_string.o:= -Wno-array-bounds
+endif
+
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) := cmdline.c
 SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_cirbuf.c
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 30b30f3..c771a85 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -125,6 +125,14 @@ CFLAGS_eal_common_options.o := -D_GNU_SOURCE
 CFLAGS_eal_common_thread.o := -D_GNU_SOURCE
 CFLAGS_eal_common_lcore.o := -D_GNU_SOURCE
 
+ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
+CFLAGS_eal_alarm.o += -Wno-parentheses-equality
+CFLAGS_eal_interrupts.o += -Wno-parentheses-equality
+CFLAGS_eal_common_options.o += -Wno-array-bounds
+CFLAGS_eal_pci.o += -Wno-parentheses-equality
+CFLAGS_eal_pci.o += -Wno-array-bounds
+endif
+
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
 ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
-- 
2.8.0.GIT

Re: [dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Bruce Richardson]

On Sun, Jun 26, 2016 at 10:54:12AM -0500, Keith Wiles wrote:
> Latest clang compiler 3.8.0 on latest update of Ubuntu
> creates a few more warnings on -Warray-bounds and extra
> () around 'if' expressions.
> 
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  app/test-pmd/Makefile                | 3 +++
>  app/test/Makefile                    | 3 +++
>  drivers/net/bonding/Makefile         | 4 ++++
>  drivers/net/fm10k/Makefile           | 2 ++
>  drivers/net/i40e/Makefile            | 2 ++
>  lib/librte_cmdline/Makefile          | 6 ++++++
>  lib/librte_eal/linuxapp/eal/Makefile | 8 ++++++++
>  7 files changed, 28 insertions(+)
> 
All the fixes in this patch seem to be just disabling the compiler warnings, which
should really be the last resort in cases like this. Can some of the issues be
fixed by actually fixing the issues in the code?

/Bruce

Re: [dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Thomas Monjalon]

2016-06-26 10:54, Keith Wiles:
> Latest clang compiler 3.8.0 on latest update of Ubuntu
> creates a few more warnings on -Warray-bounds and extra
> () around 'if' expressions.

The goal of compiler checks is to fix the code.
Why disabling these checks instead of fixing the code?
If they are false positive, they need to be justified and
limited to some compiler versions.

Re: [dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Wiles, Keith]

On 6/27/16, 3:46 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:

>On Sun, Jun 26, 2016 at 10:54:12AM -0500, Keith Wiles wrote:
>> Latest clang compiler 3.8.0 on latest update of Ubuntu
>> creates a few more warnings on -Warray-bounds and extra
>> () around 'if' expressions.
>> 
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
>>  app/test-pmd/Makefile                | 3 +++
>>  app/test/Makefile                    | 3 +++
>>  drivers/net/bonding/Makefile         | 4 ++++
>>  drivers/net/fm10k/Makefile           | 2 ++
>>  drivers/net/i40e/Makefile            | 2 ++
>>  lib/librte_cmdline/Makefile          | 6 ++++++
>>  lib/librte_eal/linuxapp/eal/Makefile | 8 ++++++++
>>  7 files changed, 28 insertions(+)
>> 
>All the fixes in this patch seem to be just disabling the compiler warnings, which
>should really be the last resort in cases like this. Can some of the issues be
>fixed by actually fixing the issues in the code?

I did look at the code to fix the problem, because I could not see one:

/work/home/rkwiles/projects/intel/dpdk/app/test-pmd/cmdline.c:3357:2140: error: array index 3 is past the end of the array (which contains 3 elements) [-Werror,-Warray-bounds]
  if (!__extension__ ({ size_t __s1_len, __s2_len; (__builtin_constant_p (res->proto) && __builtin_constant_p ("ip") && (__s1_len = __builtin_strlen (res->proto), __s2_len = __builtin_strlen ("ip"), (!((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) || __s1_len >= 4) && (!((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) || __s2_len >= 4)) ? __builtin_strcmp (res->proto, "ip") : (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) && (__s1_len = __builtin_strlen (res->proto), __s1_len < 4) ? (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) ? __builtin_strcmp (res->proto, "ip") : (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) ("ip"); int __result = (((const unsigned char *) (const char *) (res->proto))[0] - __s2[0]); if (__s1_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[1] - __s2[1]); if (__s1_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[2] - __s2[2]); if (__s1_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) (res->proto))[3] - __s2[3]); } } __result; }))) : (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) && (__s2_len = __builtin_strlen ("ip"), __s2_len < 4) ? (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) ? __builtin_strcmp (res->proto, "ip") : (- (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) (res->proto); int __result = (((const unsigned char *) (const char *) ("ip"))[0] - __s2[0]); if (__s2_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[1] - __s2[1]); if (__s2_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[2] - __s2[2]); if (__s2_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) ("ip"))[3] - __s2[3]); } } __result; })))) : __builtin_strcmp (res->proto, "ip")))); })) {

Here is the line of code for that one:
                if (!strcmp(res->proto, "ip")) {

The –Wno-parenthese-equality problem gives the output here:

/work/home/rkwiles/projects/intel/dpdk/lib/librte_cmdline/cmdline_cirbuf.c:288:19: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
 if (((cbuf)->len == 0)) {

The line is:

        if (CIRBUF_IS_EMPTY(cbuf)) {

This one is in cmdline_cirbuf.h, which can be changed, but I do not think we need to remove the parenthese.

I will look at some of other solution, so I rejected the patch.


>
>/Bruce
>

Re: [dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Wiles, Keith]

On 6/27/16, 7:58 AM, "dev on behalf of Wiles, Keith" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:

>
>On 6/27/16, 3:46 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:
>
>>On Sun, Jun 26, 2016 at 10:54:12AM -0500, Keith Wiles wrote:
>>> Latest clang compiler 3.8.0 on latest update of Ubuntu
>>> creates a few more warnings on -Warray-bounds and extra
>>> () around 'if' expressions.
>>> 
>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>>> ---
>>>  app/test-pmd/Makefile                | 3 +++
>>>  app/test/Makefile                    | 3 +++
>>>  drivers/net/bonding/Makefile         | 4 ++++
>>>  drivers/net/fm10k/Makefile           | 2 ++
>>>  drivers/net/i40e/Makefile            | 2 ++
>>>  lib/librte_cmdline/Makefile          | 6 ++++++
>>>  lib/librte_eal/linuxapp/eal/Makefile | 8 ++++++++
>>>  7 files changed, 28 insertions(+)
>>> 
>>All the fixes in this patch seem to be just disabling the compiler warnings, which
>>should really be the last resort in cases like this. Can some of the issues be
>>fixed by actually fixing the issues in the code?
>
>I did look at the code to fix the problem, because I could not see one:
>
>/work/home/rkwiles/projects/intel/dpdk/app/test-pmd/cmdline.c:3357:2140: error: array index 3 is past the end of the array (which contains 3 elements) [-Werror,-Warray-bounds]
>  if (!__extension__ ({ size_t __s1_len, __s2_len; (__builtin_constant_p (res->proto) && __builtin_constant_p ("ip") && (__s1_len = __builtin_strlen (res->proto), __s2_len = __builtin_strlen ("ip"), (!((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) || __s1_len >= 4) && (!((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) || __s2_len >= 4)) ? __builtin_strcmp (res->proto, "ip") : (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) && (__s1_len = __builtin_strlen (res->proto), __s1_len < 4) ? (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) ? __builtin_strcmp (res->proto, "ip") : (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) ("ip"); int __result = (((const unsigned char *) (const char *) (res->proto))[0] - __s2[0]); if (__s1_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[1] - __s2[1]); if (__s1_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[2] - __s2[2]); if (__s1_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) (res->proto))[3] - __s2[3]); } } __result; }))) : (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) && (__s2_len = __builtin_strlen ("ip"), __s2_len < 4) ? (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) ? __builtin_strcmp (res->proto, "ip") : (- (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) (res->proto); int __result = (((const unsigned char *) (const char *) ("ip"))[0] - __s2[0]); if (__s2_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[1] - __s2[1]); if (__s2_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[2] - __s2[2]); if (__s2_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) ("ip"))[3] - __s2[3]); } } __result; })))) : __builtin_strcmp (res->proto, "ip")))); })) {
>
>Here is the line of code for that one:
>                if (!strcmp(res->proto, "ip")) {
>
>The –Wno-parenthese-equality problem gives the output here:
>
>/work/home/rkwiles/projects/intel/dpdk/lib/librte_cmdline/cmdline_cirbuf.c:288:19: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
> if (((cbuf)->len == 0)) {
>
>The line is:
>
>        if (CIRBUF_IS_EMPTY(cbuf)) {
>
>This one is in cmdline_cirbuf.h, which can be changed, but I do not think we need to remove the parenthese.
>
>I will look at some of other solution, so I rejected the patch.

I found the problem to the compile errors I am seeing with building with clang and shared libraries.

The x86_64-linux-gnu/bits/string2.h header file if getting included from string.h, but this would be mean __GNUC__ is defined and this is the clang compiler. After much investigation it turns out ‘ccache’ is the problem here. If ccache is enabled with clang builds the __GNUC__ is defined some how, I never did find the location.

Just a warning it appears ‘ccache’ for caching object files is not compatible with DPDK builds ☹ in all cases.

>
>
>>
>>/Bruce
>>
>
>
>
>

Re: [dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Bruce Richardson]

On Mon, Jun 27, 2016 at 05:29:59PM +0100, Wiles, Keith wrote:
> On 6/27/16, 7:58 AM, "dev on behalf of Wiles, Keith" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:
> 
> >
> >On 6/27/16, 3:46 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:
> >
> >>On Sun, Jun 26, 2016 at 10:54:12AM -0500, Keith Wiles wrote:
> >>> Latest clang compiler 3.8.0 on latest update of Ubuntu
> >>> creates a few more warnings on -Warray-bounds and extra
> >>> () around 'if' expressions.
> >>> 
> >>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> >>> ---
> >>>  app/test-pmd/Makefile                | 3 +++
> >>>  app/test/Makefile                    | 3 +++
> >>>  drivers/net/bonding/Makefile         | 4 ++++
> >>>  drivers/net/fm10k/Makefile           | 2 ++
> >>>  drivers/net/i40e/Makefile            | 2 ++
> >>>  lib/librte_cmdline/Makefile          | 6 ++++++
> >>>  lib/librte_eal/linuxapp/eal/Makefile | 8 ++++++++
> >>>  7 files changed, 28 insertions(+)
> >>> 
> >>All the fixes in this patch seem to be just disabling the compiler warnings, which
> >>should really be the last resort in cases like this. Can some of the issues be
> >>fixed by actually fixing the issues in the code?
> >
> >I did look at the code to fix the problem, because I could not see one:
> >
> >/work/home/rkwiles/projects/intel/dpdk/app/test-pmd/cmdline.c:3357:2140: error: array index 3 is past the end of the array (which contains 3 elements) [-Werror,-Warray-bounds]
> >  if (!__extension__ ({ size_t __s1_len, __s2_len; (__builtin_constant_p (res->proto) && __builtin_constant_p ("ip") && (__s1_len = __builtin_strlen (res->proto), __s2_len = __builtin_strlen ("ip"), (!((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) || __s1_len >= 4) && (!((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) || __s2_len >= 4)) ? __builtin_strcmp (res->proto, "ip") : (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) && (__s1_len = __builtin_strlen (res->proto), __s1_len < 4) ? (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) ? __builtin_strcmp (res->proto, "ip") : (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) ("ip"); int __result = (((const unsigned char *) (const char *) (res->proto))[0] - __s2[0]); if (__s1_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[1] - __s2[1]); if (__s1_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[2] - __s2[2]); if (__s1_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) (res->proto))[3] - __s2[3]); } } __result; }))) : (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) && (__s2_len = __builtin_strlen ("ip"), __s2_len < 4) ? (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) ? __builtin_strcmp (res->proto, "ip") : (- (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) (res->proto); int __result = (((const unsigned char *) (const char *) ("ip"))[0] - __s2[0]); if (__s2_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[1] - __s2[1]); if (__s2_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[2] - __s2[2]); if (__s2_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) ("ip"))[3] - __s2[3]); } } __result; })))) : __builtin_strcmp (res->proto, "ip")))); })) {
> >
> >Here is the line of code for that one:
> >                if (!strcmp(res->proto, "ip")) {
> >
> >The –Wno-parenthese-equality problem gives the output here:
> >
> >/work/home/rkwiles/projects/intel/dpdk/lib/librte_cmdline/cmdline_cirbuf.c:288:19: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
> > if (((cbuf)->len == 0)) {
> >
> >The line is:
> >
> >        if (CIRBUF_IS_EMPTY(cbuf)) {
> >
> >This one is in cmdline_cirbuf.h, which can be changed, but I do not think we need to remove the parenthese.
> >
> >I will look at some of other solution, so I rejected the patch.
> 
> I found the problem to the compile errors I am seeing with building with clang and shared libraries.
> 
> The x86_64-linux-gnu/bits/string2.h header file if getting included from string.h, but this would be mean __GNUC__ is defined and this is the clang compiler. After much investigation it turns out ‘ccache’ is the problem here. If ccache is enabled with clang builds the __GNUC__ is defined some how, I never did find the location.
> 
> Just a warning it appears ‘ccache’ for caching object files is not compatible with DPDK builds ☹ in all cases.
> 
Actually, I believe it's a more general ccache and clang problem, not DPDK specific.

See e.g. http://petereisentraut.blogspot.com/2011/09/ccache-and-clang-part-2.html

where the recommendation is to set "export CCACHE_CPP2=yes" in your environment.
This cleared quite a number of issues for me (and others) when compiling with
clang.

[Credit too to Ferruh who first pointed this issue out to me]

/Bruce

Re: [dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Wiles, Keith]

On 6/27/16, 11:40 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:

>On Mon, Jun 27, 2016 at 05:29:59PM +0100, Wiles, Keith wrote:
>> On 6/27/16, 7:58 AM, "dev on behalf of Wiles, Keith" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:
>> 
>> >
>> >On 6/27/16, 3:46 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:
>> >
>> >>On Sun, Jun 26, 2016 at 10:54:12AM -0500, Keith Wiles wrote:
>> >>> Latest clang compiler 3.8.0 on latest update of Ubuntu
>> >>> creates a few more warnings on -Warray-bounds and extra
>> >>> () around 'if' expressions.
>> >>> 
>> >>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> >>> ---
>> >>>  app/test-pmd/Makefile                | 3 +++
>> >>>  app/test/Makefile                    | 3 +++
>> >>>  drivers/net/bonding/Makefile         | 4 ++++
>> >>>  drivers/net/fm10k/Makefile           | 2 ++
>> >>>  drivers/net/i40e/Makefile            | 2 ++
>> >>>  lib/librte_cmdline/Makefile          | 6 ++++++
>> >>>  lib/librte_eal/linuxapp/eal/Makefile | 8 ++++++++
>> >>>  7 files changed, 28 insertions(+)
>> >>> 
>> >>All the fixes in this patch seem to be just disabling the compiler warnings, which
>> >>should really be the last resort in cases like this. Can some of the issues be
>> >>fixed by actually fixing the issues in the code?
>> >
>> >I did look at the code to fix the problem, because I could not see one:
>> >
>> >/work/home/rkwiles/projects/intel/dpdk/app/test-pmd/cmdline.c:3357:2140: error: array index 3 is past the end of the array (which contains 3 elements) [-Werror,-Warray-bounds]
>> >  if (!__extension__ ({ size_t __s1_len, __s2_len; (__builtin_constant_p (res->proto) && __builtin_constant_p ("ip") && (__s1_len = __builtin_strlen (res->proto), __s2_len = __builtin_strlen ("ip"), (!((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) || __s1_len >= 4) && (!((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) || __s2_len >= 4)) ? __builtin_strcmp (res->proto, "ip") : (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) && (__s1_len = __builtin_strlen (res->proto), __s1_len < 4) ? (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) ? __builtin_strcmp (res->proto, "ip") : (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) ("ip"); int __result = (((const unsigned char *) (const char *) (res->proto))[0] - __s2[0]); if (__s1_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[1] - __s2[1]); if (__s1_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) (res->proto))[2] - __s2[2]); if (__s1_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) (res->proto))[3] - __s2[3]); } } __result; }))) : (__builtin_constant_p ("ip") && ((size_t)(const void *)(("ip") + 1) - (size_t)(const void *)("ip") == 1) && (__s2_len = __builtin_strlen ("ip"), __s2_len < 4) ? (__builtin_constant_p (res->proto) && ((size_t)(const void *)((res->proto) + 1) - (size_t)(const void *)(res->proto) == 1) ? __builtin_strcmp (res->proto, "ip") : (- (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) (res->proto); int __result = (((const unsigned char *) (const char *) ("ip"))[0] - __s2[0]); if (__s2_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[1] - __s2[1]); if (__s2_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) ("ip"))[2] - __s2[2]); if (__s2_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) ("ip"))[3] - __s2[3]); } } __result; })))) : __builtin_strcmp (res->proto, "ip")))); })) {
>> >
>> >Here is the line of code for that one:
>> >                if (!strcmp(res->proto, "ip")) {
>> >
>> >The –Wno-parenthese-equality problem gives the output here:
>> >
>> >/work/home/rkwiles/projects/intel/dpdk/lib/librte_cmdline/cmdline_cirbuf.c:288:19: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
>> > if (((cbuf)->len == 0)) {
>> >
>> >The line is:
>> >
>> >        if (CIRBUF_IS_EMPTY(cbuf)) {
>> >
>> >This one is in cmdline_cirbuf.h, which can be changed, but I do not think we need to remove the parenthese.
>> >
>> >I will look at some of other solution, so I rejected the patch.
>> 
>> I found the problem to the compile errors I am seeing with building with clang and shared libraries.
>> 
>> The x86_64-linux-gnu/bits/string2.h header file if getting included from string.h, but this would be mean __GNUC__ is defined and this is the clang compiler. After much investigation it turns out ‘ccache’ is the problem here. If ccache is enabled with clang builds the __GNUC__ is defined some how, I never did find the location.
>> 
>> Just a warning it appears ‘ccache’ for caching object files is not compatible with DPDK builds ☹ in all cases.
>> 
>Actually, I believe it's a more general ccache and clang problem, not DPDK specific.
>
>See e.g. http://petereisentraut.blogspot.com/2011/09/ccache-and-clang-part-2.html
>
>where the recommendation is to set "export CCACHE_CPP2=yes" in your environment.
>This cleared quite a number of issues for me (and others) when compiling with
>clang.

Adding the CCACHE_CPP2=yes

Cleaned up all of the compiler errors I was seeing ☺

Do we need to document this in DPDK and if so where in the docs would someone suggest it be placed?

>
>[Credit too to Ferruh who first pointed this issue out to me]
>
>/Bruce
>

Re: [dpdk-dev] [PATCH v2 2/2] fix building with clang-3.8.0 compiler

[Thomas Monjalon]

2016-06-27 16:51, Wiles, Keith:
> On 6/27/16, 11:40 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:
> >On Mon, Jun 27, 2016 at 05:29:59PM +0100, Wiles, Keith wrote:
> >> On 6/27/16, 7:58 AM, on behalf of keith.wiles@intel.com> wrote:
> >> >On 6/27/16, 3:46 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:
> >> >>On Sun, Jun 26, 2016 at 10:54:12AM -0500, Keith Wiles wrote:
> >> I found the problem to the compile errors I am seeing with building with clang and shared libraries.
> >> 
> >> The x86_64-linux-gnu/bits/string2.h header file if getting included from string.h, but this would be mean __GNUC__ is defined and this is the clang compiler. After much investigation it turns out ‘ccache’ is the problem here. If ccache is enabled with clang builds the __GNUC__ is defined some how, I never did find the location.
> >> 
> >> Just a warning it appears ‘ccache’ for caching object files is not compatible with DPDK builds ☹ in all cases.
> >> 
> >Actually, I believe it's a more general ccache and clang problem, not DPDK specific.
> >
> >See e.g. http://petereisentraut.blogspot.com/2011/09/ccache-and-clang-part-2.html
> >
> >where the recommendation is to set "export CCACHE_CPP2=yes" in your environment.
> >This cleared quite a number of issues for me (and others) when compiling with
> >clang.
> 
> Adding the CCACHE_CPP2=yes
> 
> Cleaned up all of the compiler errors I was seeing ☺
> 
> Do we need to document this in DPDK and if so where in the docs would someone suggest it be placed?

I do not use ccache with clang because it is not known to work.
But if you are sure it can work with few configuration, I would be glad
to have a complete description.
However it is not related to DPDK at all. That's why a nice email well
written and explained on this ml looks enough to be found via Google.
Thanks