diff mbox

batctl: suppress implicit-fallthrough compiler warning

Message ID 20170613110824.27786-2-philipp.psurek@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Simon Wunderlich
Headers

Commit Message

Philipp Psurek June 13, 2017, 11:08 a.m. UTC 
  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(-)

Comments

Simon Wunderlich June 13, 2017, 11:46 a.m. UTC | #1 
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
Sven Eckelmann June 13, 2017, 11:53 a.m. UTC | #2 
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
Sven Eckelmann June 13, 2017, 11:55 a.m. UTC | #3 
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
Philipp Psurek June 13, 2017, 12:25 p.m. UTC | #4 
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
diff mbox

Patch

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;