b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings
@ 2017-05-29 17:09 Philipp Psurek
  2017-06-01  8:34 ` Simon Wunderlich
  2017-06-13  8:25 ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
  0 siblings, 2 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-05-29 17:09 UTC (permalink / raw)
  To: b.a.t.m.a.n

Hi all,

it's nothing to worry about but since 2011 I always noticed a clean
compile run of your code. Now with gcc 7.1.0 there are some warnings
and notes that might interest you:

~ LANG=C make
    CC bat-hosts.o
    CC debugfs.o
    CC debug.o
    CC functions.o
    CC genl.o
    CC hash.o
    CC icmp_helper.o
    CC interface.o
    CC ioctl.o
    CC main.o
    CC netlink.o
    CC ping.o
    CC sys.o
In file included from sys.c:37:0:
sys.c: In function 'handle_ra_setting':
sys.h:33:25: warning: '%s' directive output may be truncated writing up to 255 bytes into a region of size 185 [-Wformat-truncation=]
 #define SYS_IFACE_PATH  "/sys/class/net"
                         ^
sys.h:38:30: note: in expansion of macro 'SYS_IFACE_PATH'
 #define SYS_ROUTING_ALGO_FMT SYS_IFACE_PATH"/%s/mesh/routing_algo"
                              ^~~~~~~~~~~~~~
sys.c:480:38: note: in expansion of macro 'SYS_ROUTING_ALGO_FMT'
   snprintf(path_buff, PATH_BUFF_LEN, SYS_ROUTING_ALGO_FMT, iface_dir->d_name);
                                      ^~~~~~~~~~~~~~~~~~~~
sys.h:38:46: note: format string is defined here
 #define SYS_ROUTING_ALGO_FMT SYS_IFACE_PATH"/%s/mesh/routing_algo"
                                              ^~
sys.c:480:3: note: 'snprintf' output between 34 and 289 bytes into a destination of size 200
   snprintf(path_buff, PATH_BUFF_LEN, SYS_ROUTING_ALGO_FMT, iface_dir->d_name);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    CC tcpdump.o
    CC tp_meter.o
tp_meter.c: In function 'tp_meter':
tp_meter.c:502:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
   printf("CANCEL received: test aborted\n");
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tp_meter.c:504:2: note: here
  case BATADV_TP_REASON_COMPLETE:
  ^~~~
    CC traceroute.o
    CC translate.o
    LD batctl

# batctl -v
batctl 2017.1-1-g3069ca8 [batman-adv: 2017.1-4-g2149d80d]

It's really nothing bad at all. gcc 7.1.0 is somehow a little bit
capricious but with more suggestions to make cleaner code.

best regards

Philipp

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings
  2017-05-29 17:09 [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
@ 2017-06-01  8:34 ` Simon Wunderlich
  2017-06-03  2:14   ` Philipp Psurek
                     ` (2 more replies)
  2017-06-13  8:25 ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
  1 sibling, 3 replies; 21+ messages in thread
From: Simon Wunderlich @ 2017-06-01  8:34 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]

On Monday, May 29, 2017 7:09:09 PM CEST Philipp Psurek wrote:
> Hi all,
> 
> it's nothing to worry about but since 2011 I always noticed a clean
> compile run of your code. Now with gcc 7.1.0 there are some warnings
> and notes that might interest you:
> 
> ~ LANG=C make
>     CC bat-hosts.o
>     CC debugfs.o
>     CC debug.o
>     CC functions.o
>     CC genl.o
>     CC hash.o
>     CC icmp_helper.o
>     CC interface.o
>     CC ioctl.o
>     CC main.o
>     CC netlink.o
>     CC ping.o
>     CC sys.o
> In file included from sys.c:37:0:
> sys.c: In function 'handle_ra_setting':
> sys.h:33:25: warning: '%s' directive output may be truncated writing up to
> 255 bytes into a region of size 185 [-Wformat-truncation=] #define
> SYS_IFACE_PATH  "/sys/class/net"
>                          ^
> sys.h:38:30: note: in expansion of macro 'SYS_IFACE_PATH'
>  #define SYS_ROUTING_ALGO_FMT SYS_IFACE_PATH"/%s/mesh/routing_algo"
>                               ^~~~~~~~~~~~~~
> sys.c:480:38: note: in expansion of macro 'SYS_ROUTING_ALGO_FMT'
>    snprintf(path_buff, PATH_BUFF_LEN, SYS_ROUTING_ALGO_FMT,
> iface_dir->d_name); ^~~~~~~~~~~~~~~~~~~~
> sys.h:38:46: note: format string is defined here
>  #define SYS_ROUTING_ALGO_FMT SYS_IFACE_PATH"/%s/mesh/routing_algo"
>                                               ^~
> sys.c:480:3: note: 'snprintf' output between 34 and 289 bytes into a
> destination of size 200 snprintf(path_buff, PATH_BUFF_LEN,
> SYS_ROUTING_ALGO_FMT, iface_dir->d_name);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> CC tcpdump.o
>     CC tp_meter.o
> tp_meter.c: In function 'tp_meter':
> tp_meter.c:502:3: warning: this statement may fall through
> [-Wimplicit-fallthrough=] printf("CANCEL received: test aborted\n");
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> tp_meter.c:504:2: note: here
>   case BATADV_TP_REASON_COMPLETE:
>   ^~~~
>     CC traceroute.o
>     CC translate.o
>     LD batctl
> 
> # batctl -v
> batctl 2017.1-1-g3069ca8 [batman-adv: 2017.1-4-g2149d80d]
> 
> It's really nothing bad at all. gcc 7.1.0 is somehow a little bit
> capricious but with more suggestions to make cleaner code.
> 
Hi Philipp,

thanks for showing this. I think none of that is really critical, and the fall 
through is actually intended.

Do you want to propose a patch for these things?

Cheers,
    Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings
  2017-06-01  8:34 ` Simon Wunderlich
@ 2017-06-03  2:14   ` Philipp Psurek
  2017-06-03  3:12   ` [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
  2017-06-03  3:40   ` [B.A.T.M.A.N.] [PATCH] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
  2 siblings, 0 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-06-03  2:14 UTC (permalink / raw)
  To: Simon Wunderlich, b.a.t.m.a.n

Hi Simon

Am Donnerstag, den 01.06.2017, 10:34 +0200 schrieb Simon Wunderlich:
> I think none of that is really critical, and the fall through is
> actually intended.

Yes, this isn't critical. This report was FYI about an exotic compiler.

> Do you want to propose a patch for these things?

Give me some time to understand the code and I look what I can do in my
spare time. This might take some time.

best regards

Philipp

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-01  8:34 ` Simon Wunderlich
  2017-06-03  2:14   ` Philipp Psurek
@ 2017-06-03  3:12   ` Philipp Psurek
  2017-06-03 10:13     ` Philipp Psurek
  2017-06-03  3:40   ` [B.A.T.M.A.N.] [PATCH] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
  2 siblings, 1 reply; 21+ messages in thread
From: Philipp Psurek @ 2017-06-03  3:12 UTC (permalink / raw)
  To: Simon Wunderlich, b.a.t.m.a.n

__attribute__ ((fallthrough)) tells the compiler to not complain about
an intended fallthrough.





diff --git a/tp_meter.c b/tp_meter.c
index 918fb79..592a9ed 100644
--- a/tp_meter.c
+++ b/tp_meter.c
@@ -501,6 +501,7 @@ int tp_meter(char *mesh_iface, int argc, char **argv)
 	case BATADV_TP_REASON_CANCEL:
 		printf("CANCEL received: test aborted\n");
 		/* fall through and print the partial result */
+		__attribute__ ((fallthrough));
 	case BATADV_TP_REASON_COMPLETE:
 		if (result.test_time > 0) {
 			throughput = result.total_bytes * 1000;


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: change PATH_BUFF_LEN to maximal possible value
  2017-06-01  8:34 ` Simon Wunderlich
  2017-06-03  2:14   ` Philipp Psurek
  2017-06-03  3:12   ` [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
@ 2017-06-03  3:40   ` Philipp Psurek
  2 siblings, 0 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-06-03  3:40 UTC (permalink / raw)
  To: Simon Wunderlich, b.a.t.m.a.n

'snprintf' output in sys.c can be between 34 and 289 bytes and should
not write into a destination of size 200

snprintf(path_buff, PATH_BUFF_LEN, SYS_ROUTING_ALGO_FMT, iface_dir->d_name);






diff --git a/functions.c b/functions.c
index 2239440..8337ee8 100644
--- a/functions.c
+++ b/functions.c
@@ -59,7 +59,7 @@
 #include "debugfs.h"
 #include "netlink.h"
 
-#define PATH_BUFF_LEN 200
+#define PATH_BUFF_LEN 289
 
 static struct timespec start_time;
 static char *host_name;
diff --git a/functions.h b/functions.h
index eca1406..c0a02a8 100644
--- a/functions.h
+++ b/functions.h
@@ -31,7 +31,7 @@
 #define ETH_STR_LEN 17
 #define BATMAN_ADV_TAG "batman-adv:"
 
-#define PATH_BUFF_LEN 200
+#define PATH_BUFF_LEN 289
 
 /* return time delta from start to end in milliseconds */
 void start_timer(void);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-03  3:12   ` [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
@ 2017-06-03 10:13     ` Philipp Psurek
  2017-06-03 10:31       ` Philipp Psurek
  0 siblings, 1 reply; 21+ messages in thread
From: Philipp Psurek @ 2017-06-03 10:13 UTC (permalink / raw)
  To: Simon Wunderlich, b.a.t.m.a.n

Well, now it's a little bit tricky to suppress compiler warnings as
anything below 7.1.0 dislike “__attribute__ ((fallthrough));” with a
warning. One could tell the 7.1.0 with “-Wimplicit-fallthrough=2” to
suppress the warning because there is a “falls?[ \t-]*thr(ough|u)”
comment, but other compiles drop an unrecognised command line option
error.

This one is not possible:

diff --git a/tp_meter.c b/tp_meter.c
index 592a9ed..c1587cd 100644
--- a/tp_meter.c
+++ b/tp_meter.c
@@ -501,7 +501,9 @@ int tp_meter(char *mesh_iface, int argc, char **argv)
 	case BATADV_TP_REASON_CANCEL:
 		printf("CANCEL received: test aborted\n");
 		/* fall through and print the partial result */
+		#if (__STDC_VERSION__ == 201112L)
+		__attribute__ ((fallthrough));
+		#endif
 	case BATADV_TP_REASON_COMPLETE:
 		if (result.test_time > 0) {
 			throughput = result.total_bytes * 1000;

because batctl is compiled with “-std=gnu99” which is predefined in the
makefile. And I don't know if you like such code.

What actually works is this hack in the makefile:

--- a/Makefile
+++ b/Makefile
@@ -101,6 +101,11 @@ MKDIR ?= mkdir -p
 COMPILE.c = $(Q_CC)$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c
 LINK.o = $(Q_LD)$(CC) $(CFLAGS) $(LDFLAGS) $(TARGET_ARCH)
 
+# Check for GCC >=7
+ifeq ($(shell $(CC) -x c -E -P - <<< __STDC_VERSION__),201112L)
+CFLAGS += -Wimplicit-fallthrough=2
+endif
+
 # standard install paths
 PREFIX = /usr/local
 SBINDIR = $(PREFIX)/sbin

I suggest to modify the makefile with this patch and make comments
on intended fallthroughs like you already did.

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-03 10:13     ` Philipp Psurek
@ 2017-06-03 10:31       ` Philipp Psurek
  0 siblings, 0 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-06-03 10:31 UTC (permalink / raw)
  To: Simon Wunderlich, b.a.t.m.a.n

OK, sorry for the many posts ... there was some trouble recognizing the
used compiler with the last patch. This now really do the work:

diff --git a/Makefile b/Makefile
index 6bebb7d..7123d83 100755
--- a/Makefile
+++ b/Makefile
@@ -101,6 +101,11 @@ MKDIR ?= mkdir -p
 COMPILE.c = $(Q_CC)$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c
 LINK.o = $(Q_LD)$(CC) $(CFLAGS) $(LDFLAGS) $(TARGET_ARCH)
 
+# Check for GCC >=7
+ifeq ($(shell $(CC) -x c++ --std=c++17 -E -P - <<< __cplusplus),201703L)
+	CFLAGS += -Wimplicit-fallthrough=2
+endif
+
 # standard install paths
 PREFIX = /usr/local
 SBINDIR = $(PREFIX)/sbin


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings
  2017-05-29 17:09 [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
  2017-06-01  8:34 ` Simon Wunderlich
@ 2017-06-13  8:25 ` Philipp Psurek
  2017-06-13  8:25   ` [B.A.T.M.A.N.] [PATCH 1/2] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
                     ` (2 more replies)
  1 sibling, 3 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13  8:25 UTC (permalink / raw)
  To: b.a.t.m.a.n

Hi Simon,

I assume my patches were not formatted according to the rules and this is the
reason why there has been no comment on them. I hope the patches are now
more satisfying.

best regards

Philipp


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [B.A.T.M.A.N.] [PATCH 1/2] batctl: change PATH_BUFF_LEN to maximal possible value
  2017-06-13  8:25 ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
@ 2017-06-13  8:25   ` Philipp Psurek
  2017-06-13  8:49     ` Simon Wunderlich
  2017-06-13  8:26   ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
  2017-06-13  8:42   ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Simon Wunderlich
  2 siblings, 1 reply; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13  8:25 UTC (permalink / raw)
  To: b.a.t.m.a.n

The output of

snprintf(path_buff, PATH_BUFF_LEN, SYS_ROUTING_ALGO_FMT, iface_dir->d_name)

in sys.c can be between 34 and 289 bytes and should not write into a
destination of size 200.

Signed-off-by: Philipp Psurek <philipp.psurek@gmail.com>
---
 functions.c | 2 +-
 functions.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/functions.c b/functions.c
index 2239440..8337ee8 100644
--- a/functions.c
+++ b/functions.c
@@ -59,7 +59,7 @@
 #include "debugfs.h"
 #include "netlink.h"
 
-#define PATH_BUFF_LEN 200
+#define PATH_BUFF_LEN 289
 
 static struct timespec start_time;
 static char *host_name;
diff --git a/functions.h b/functions.h
index eca1406..c0a02a8 100644
--- a/functions.h
+++ b/functions.h
@@ -31,7 +31,7 @@
 #define ETH_STR_LEN 17
 #define BATMAN_ADV_TAG "batman-adv:"
 
-#define PATH_BUFF_LEN 200
+#define PATH_BUFF_LEN 289
 
 /* return time delta from start to end in milliseconds */
 void start_timer(void);
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13  8:25 ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
  2017-06-13  8:25   ` [B.A.T.M.A.N.] [PATCH 1/2] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
@ 2017-06-13  8:26   ` Philipp Psurek
  2017-06-13  8:48     ` Simon Wunderlich
  2017-06-13  8:42   ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Simon Wunderlich
  2 siblings, 1 reply; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13  8:26 UTC (permalink / raw)
  To: b.a.t.m.a.n

GCC 7.1.0 complains about an intended fallthrough.
“__attribute__ ((fallthrough))” in this part of code would suppress this
warning. Because older GCC compiler don’t understand this statement attribute
and because there is already a comment in the source containing
“falls?[ \t-]*thr(ough|u)” we can suppress the warning with the
“-Wimplicit-fallthrough=2” warning option. Unintended fallthroughs without a
comment will trigger this warning.

Older GCC compiler don’t understand this warning option and exits with error.
There has to be a compiler recognition after that only GCC =>7.1.0 receives
this option.

Signed-off-by: Philipp Psurek <philipp.psurek@gmail.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 6bebb7d..29b7e2f 100755
--- a/Makefile
+++ b/Makefile
@@ -101,6 +101,11 @@ MKDIR ?= mkdir -p
 COMPILE.c = $(Q_CC)$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c
 LINK.o = $(Q_LD)$(CC) $(CFLAGS) $(LDFLAGS) $(TARGET_ARCH)
 
+# Check for GCC >=7
+ifeq ($(shell $(CC) -x c++ --std=c++17 -E -P - <<< __cplusplus),201703L)
+       CFLAGS += -Wimplicit-fallthrough=2
+endif
+
 # standard install paths
 PREFIX = /usr/local
 SBINDIR = $(PREFIX)/sbin
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings
  2017-06-13  8:25 ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
  2017-06-13  8:25   ` [B.A.T.M.A.N.] [PATCH 1/2] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
  2017-06-13  8:26   ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
@ 2017-06-13  8:42   ` Simon Wunderlich
  2 siblings, 0 replies; 21+ messages in thread
From: Simon Wunderlich @ 2017-06-13  8:42 UTC (permalink / raw)
  To: Philipp Psurek; +Cc: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Hi Philipp,

sorry for the late reply - I was out at battlemesh last week, so I didn't had 
time to check them. But yes, the format was lacking indeed, thanks for fixing 
that. :)

I'll check the patches and reply to them directly.

Thanks,
    Simon

On Tuesday, June 13, 2017 10:25:58 AM CEST Philipp Psurek wrote:
> Hi Simon,
> 
> I assume my patches were not formatted according to the rules and this is
> the reason why there has been no comment on them. I hope the patches are
> now more satisfying.
> 
> best regards
> 
> Philipp


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13  8:26   ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
@ 2017-06-13  8:48     ` Simon Wunderlich
  2017-06-13 10:39       ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler Philipp Psurek
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Wunderlich @ 2017-06-13  8:48 UTC (permalink / raw)
  To: Philipp Psurek; +Cc: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

On Tuesday, June 13, 2017 10:26:00 AM CEST Philipp Psurek wrote:
> GCC 7.1.0 complains about an intended fallthrough.
> “__attribute__ ((fallthrough))” in this part of code would suppress this
> warning. Because older GCC compiler don’t understand this statement
> attribute and because there is already a comment in the source containing
> “falls?[ \t-]*thr(ough|u)” we can suppress the warning with the
> “-Wimplicit-fallthrough=2” warning option. Unintended fallthroughs without a
> comment will trigger this warning.
> 
> Older GCC compiler don’t understand this warning option and exits with
> error. There has to be a compiler recognition after that only GCC =>7.1.0
> receives this option.
> 
> Signed-off-by: Philipp Psurek <philipp.psurek@gmail.com>
> ---
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 6bebb7d..29b7e2f 100755
> --- a/Makefile
> +++ b/Makefile
> @@ -101,6 +101,11 @@ MKDIR ?= mkdir -p
>  COMPILE.c = $(Q_CC)$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c
>  LINK.o = $(Q_LD)$(CC) $(CFLAGS) $(LDFLAGS) $(TARGET_ARCH)
> 
> +# Check for GCC >=7
> +ifeq ($(shell $(CC) -x c++ --std=c++17 -E -P - <<< __cplusplus),201703L)
> +       CFLAGS += -Wimplicit-fallthrough=2
> +endif
> +

Hmm, I don't like this one too much, since it looks cryptic and we will have 
to add up new versions, which I'd like to avoid.

Did you try to change the wording? I've googled a bit and it seems a comment 
containing the wording "fallthrough" will avoid it. Maybe we should just 
change "fall through" to "fallthrough"?

Thanks,
      Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 1/2] batctl: change PATH_BUFF_LEN to maximal possible value
  2017-06-13  8:25   ` [B.A.T.M.A.N.] [PATCH 1/2] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
@ 2017-06-13  8:49     ` Simon Wunderlich
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Wunderlich @ 2017-06-13  8:49 UTC (permalink / raw)
  To: Philipp Psurek; +Cc: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

On Tuesday, June 13, 2017 10:25:59 AM CEST Philipp Psurek wrote:
> The output of
> 
> snprintf(path_buff, PATH_BUFF_LEN, SYS_ROUTING_ALGO_FMT, iface_dir->d_name)
> 
> in sys.c can be between 34 and 289 bytes and should not write into a
> destination of size 200.
> 
> Signed-off-by: Philipp Psurek <philipp.psurek@gmail.com>

I've picked this one, but increased the size to 400 to be a bit more future 
proof. :)

Thanks,
      Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler
  2017-06-13  8:48     ` Simon Wunderlich
@ 2017-06-13 10:39       ` Philipp Psurek
  2017-06-13 10:39         ` [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
  2017-06-13 11:08         ` [B.A.T.M.A.N.] [PATCH 2/2] " Philipp Psurek
  0 siblings, 2 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13 10:39 UTC (permalink / raw)
  To: b.a.t.m.a.n

Am Dienstag, den 13.06.2017, 10:48 +0200 schrieb Simon Wunderlich:
> Hmm, I don't like this one too much, since it looks cryptic and we
> will have to add up new versions, which I'd like to avoid.
> 
> Did you try to change the wording? I've googled a bit and it seems a
> comment containing the wording "fallthrough" will avoid it. Maybe we should
> just change "fall through" to "fallthrough"?

I also read about the wording but for some reason I do not want to change the
comment; because the many changes I tried were not recognized without a
compiler warning option in conjunction with the complete comment. The now
provided patch is recognized but the comment looks a little bit messed up. It
works if you don't mind the formatting of the code.

Thanks for the opportunity to contribute something to B.A.T.M.A.N.-Advanced.

Best regards,

Philipp


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13 10:39       ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler Philipp Psurek
@ 2017-06-13 10:39         ` Philipp Psurek
  2017-06-13 11:08         ` [B.A.T.M.A.N.] [PATCH 2/2] " Philipp Psurek
  1 sibling, 0 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13 10:39 UTC (permalink / raw)
  To: b.a.t.m.a.n

GCC 7.1.0 complains about an intended fallthrough.
“__attribute__ ((fallthrough))” in this part of code would suppress this
warning. Because older GCC compiler don’t understand this statement attribute
and because there is already a comment in the source containing
“falls?[ \t-]*thr(ough|u)” we can suppress the warning with the
“-Wimplicit-fallthrough=2” warning option. Unintended fallthroughs without a
comment would trigger this warning again.

To avoid compiler recognition in the Makefile a simply change of the comment
is sufficient to suppress the warning. For some reason only stand alone
comments mentioned in [1] are recognized so the comment has to be split up into
two parts.

[1] https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Warning-Options.html#index-Wimplicit-fallthrough_003d

Signed-off-by: Philipp Psurek <philipp.psurek@gmail.com>
---
 tp_meter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tp_meter.c b/tp_meter.c
index 918fb79..bd7fdb4 100644
--- a/tp_meter.c
+++ b/tp_meter.c
@@ -500,7 +500,7 @@ int tp_meter(char *mesh_iface, int argc, char **argv)
 		break;
 	case BATADV_TP_REASON_CANCEL:
 		printf("CANCEL received: test aborted\n");
-		/* fall through and print the partial result */
+		/* fallthrough *//* and print the partial result */
 	case BATADV_TP_REASON_COMPLETE:
 		if (result.test_time > 0) {
 			throughput = result.total_bytes * 1000;
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13 10:39       ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler Philipp Psurek
  2017-06-13 10:39         ` [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
@ 2017-06-13 11:08         ` Philipp Psurek
  2017-06-13 11:08           ` [B.A.T.M.A.N.] [PATCH] " Philipp Psurek
  1 sibling, 1 reply; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13 11:08 UTC (permalink / raw)
  To: b.a.t.m.a.n

Sorry for the many posts but this one works also; the comment has to be 
definitely split up into two parts if you do not want to append warning options
to the compiler.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13 11:08         ` [B.A.T.M.A.N.] [PATCH 2/2] " Philipp Psurek
@ 2017-06-13 11:08           ` Philipp Psurek
  2017-06-13 11:46             ` Simon Wunderlich
  0 siblings, 1 reply; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13 11:08 UTC (permalink / raw)
  To: b.a.t.m.a.n

GCC 7.1.0 complains about an intended fallthrough.
“__attribute__ ((fallthrough))” in this part of code would suppress this
warning. Because older GCC compiler don’t understand this statement attribute
and because there is already a comment in the source containing
“falls?[ \t-]*thr(ough|u)” we can suppress the warning with the
“-Wimplicit-fallthrough=2” warning option. Unintended fallthroughs without a
comment would trigger this warning again.

To avoid compiler recognition in the Makefile a simply change of the comment
is sufficient to suppress the warning. For some reason only stand alone
comments mentioned in [1] are recognized so the comment has to be split up into
two parts.

[1] https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Warning-Options.html#index-Wimplicit-fallthrough_003d

Signed-off-by: Philipp Psurek <philipp.psurek@gmail.com>
---
 tp_meter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tp_meter.c b/tp_meter.c
index 918fb79..eb67ba1 100644
--- a/tp_meter.c
+++ b/tp_meter.c
@@ -500,7 +500,7 @@ int tp_meter(char *mesh_iface, int argc, char **argv)
 		break;
 	case BATADV_TP_REASON_CANCEL:
 		printf("CANCEL received: test aborted\n");
-		/* fall through and print the partial result */
+		/* fall through *//* and print the partial result */
 	case BATADV_TP_REASON_COMPLETE:
 		if (result.test_time > 0) {
 			throughput = result.total_bytes * 1000;
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13 11:08           ` [B.A.T.M.A.N.] [PATCH] " Philipp Psurek
@ 2017-06-13 11:46             ` Simon Wunderlich
  2017-06-13 11:53               ` Sven Eckelmann
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Wunderlich @ 2017-06-13 11:46 UTC (permalink / raw)
  To: Philipp Psurek; +Cc: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]

On Tuesday, June 13, 2017 1:08:24 PM CEST Philipp Psurek wrote:
> GCC 7.1.0 complains about an intended fallthrough.
> “__attribute__ ((fallthrough))” in this part of code would suppress this
> warning. Because older GCC compiler don’t understand this statement
> attribute and because there is already a comment in the source containing
> “falls?[ \t-]*thr(ough|u)” we can suppress the warning with the
> “-Wimplicit-fallthrough=2” warning option. Unintended fallthroughs without a
> comment would trigger this warning again.
> 
> To avoid compiler recognition in the Makefile a simply change of the comment
> is sufficient to suppress the warning. For some reason only stand alone
> comments mentioned in [1] are recognized so the comment has to be split up
> into two parts.
> 
> [1]
> https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Warning-Options.html#index-Wim
> plicit-fallthrough_003d
> 
> Signed-off-by: Philipp Psurek <philipp.psurek@gmail.com>

Thank you for your persistence and testing!

I've adopted your patch, but put the second part of the comment into the 
following case to avoid the ugliness. I didn't test with gcc 7.1 but hope that 
should work. Please check here:

https://git.open-mesh.org/batctl.git/blobdiff/
620226bf8cff30e6dd966c8fe922b2d4cddf843b..
50ee3c45feeda6d8c04ee127097badf99f78a26e:/tp_meter.c

Thank you!
      Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13 11:46             ` Simon Wunderlich
@ 2017-06-13 11:53               ` Sven Eckelmann
  2017-06-13 11:55                 ` Sven Eckelmann
  0 siblings, 1 reply; 21+ messages in thread
From: Sven Eckelmann @ 2017-06-13 11:53 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

On Dienstag, 13. Juni 2017 13:46:04 CEST Simon Wunderlich wrote:
[...]
> I've adopted your patch, but put the second part of the comment into the 
> following case to avoid the ugliness. I didn't test with gcc 7.1 but hope that 
> should work. Please check here:
> 
> https://git.open-mesh.org/batctl.git/blobdiff/
> 620226bf8cff30e6dd966c8fe922b2d4cddf843b..
> 50ee3c45feeda6d8c04ee127097badf99f78a26e:/tp_meter.c

The other option would have been to change it to:

    /* fall through - print the partial result */

At least the documentation allows non-escape chars after the "fall through" 
marker when a "-" was added between them. I've removed the "and" in this
example because it looked weird.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13 11:53               ` Sven Eckelmann
@ 2017-06-13 11:55                 ` Sven Eckelmann
  2017-06-13 12:25                   ` Philipp Psurek
  0 siblings, 1 reply; 21+ messages in thread
From: Sven Eckelmann @ 2017-06-13 11:55 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

On Dienstag, 13. Juni 2017 13:53:01 CEST Sven Eckelmann wrote:
> At least the documentation allows non-escape chars after the "fall through" 

Sry, I meant "non-newline"/"non-return" chars.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning
  2017-06-13 11:55                 ` Sven Eckelmann
@ 2017-06-13 12:25                   ` Philipp Psurek
  0 siblings, 0 replies; 21+ messages in thread
From: Philipp Psurek @ 2017-06-13 12:25 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

Am Dienstag, den 13.06.2017, 13:53 +0200 schrieb Sven Eckelmann:
> The other option would have been to change it to:
> 
>     /* fall through - print the partial result */

Thank you Sven for pointing this out. This one works perfectly! Regex
understanding 4TW ;-)

Thank you Simon for restyling the patch.

Batctl compiles now without any warnings with GCC 7.1.0. I’m surprised
how far it has come: the compiler inspects comments now …

Thank you all for B.A.T.M.A.N.-Advanced

Best regards,

Philipp

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-06-13 12:25 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-29 17:09 [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
2017-06-01  8:34 ` Simon Wunderlich
2017-06-03  2:14   ` Philipp Psurek
2017-06-03  3:12   ` [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
2017-06-03 10:13     ` Philipp Psurek
2017-06-03 10:31       ` Philipp Psurek
2017-06-03  3:40   ` [B.A.T.M.A.N.] [PATCH] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
2017-06-13  8:25 ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Philipp Psurek
2017-06-13  8:25   ` [B.A.T.M.A.N.] [PATCH 1/2] batctl: change PATH_BUFF_LEN to maximal possible value Philipp Psurek
2017-06-13  8:49     ` Simon Wunderlich
2017-06-13  8:26   ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
2017-06-13  8:48     ` Simon Wunderlich
2017-06-13 10:39       ` [B.A.T.M.A.N.] [PATCH 2/2] batctl: suppress implicit-fallthrough compiler Philipp Psurek
2017-06-13 10:39         ` [B.A.T.M.A.N.] [PATCH] batctl: suppress implicit-fallthrough compiler warning Philipp Psurek
2017-06-13 11:08         ` [B.A.T.M.A.N.] [PATCH 2/2] " Philipp Psurek
2017-06-13 11:08           ` [B.A.T.M.A.N.] [PATCH] " Philipp Psurek
2017-06-13 11:46             ` Simon Wunderlich
2017-06-13 11:53               ` Sven Eckelmann
2017-06-13 11:55                 ` Sven Eckelmann
2017-06-13 12:25                   ` Philipp Psurek
2017-06-13  8:42   ` [B.A.T.M.A.N.] [batctl] compiling with gcc 7.1.0: there are some notes and warnings Simon Wunderlich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).