make check fails with gcc7

[ghost]

Due to the new implicit-fallthrough warning and -Werror make check fails with gcc7.

make  tests/pick-test
make[1]: Entering directory '/build/pick/src/pick-1.7.0'
gcc -DHAVE_CONFIG_H -I.  -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -pedantic -Wall -Werror -Wextra -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -MT tests/tests_pick_test-pick-test.o -MD -MP -MF tests/.deps/tests_pick_test-pick-test.Tpo -c -o tests/tests_pick_test-pick-test.o `test -f 'tests/pick-test.c' || echo './'`tests/pick-test.c
tests/pick-test.c: In function ‘main’:
tests/pick-test.c:76:3: error: this statement may fall through [-Werror=implicit-fallthrough=]
   child(master, slave);
   ^~~~~~~~~~~~~~~~~~~~
tests/pick-test.c:78:2: note: here
  default:
  ^~~~~~~
cc1: all warnings being treated as errors

[ghost]

Seems like gcc7 does not recognize the /* NOTREACHED */-comment from the old lint to suppress the warning, so there should be an unnecessary break; after child(master, slave);

[ghost]

so there should be an unnecessary break; after child(master, slave);

That, or adding -Wimplicit-fallthrough=1 to the CFLAGS.

From: https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/

The Warning is enabled by -Wextra

-Wimplicit-fallthrough=0 disables the warning altogether.
-Wimplicit-fallthrough=1 treats any kind of comment as a “falls through” comment.

[ghost]

That, or adding -Wimplicit-fallthrough=1 to the CFLAGS.

I'm for -Wimplicit-fallthrough=0 because it is not really a "falls through" ... although adding new switches to CFLAGS without checking the version of gcc leads to errors on older ones ...

[mptre]

Does #204 solve this issue?

[ghost]

Sorry to report, Anton, it does not ... (gcc (Debian 7.1.0-7) 7.1.0)

[mptre]

Is __dead defined on Debian?

diff --git a/compat.h b/compat.h
index 6d2cfee..99ef9f9 100644
--- a/compat.h
+++ b/compat.h
@@ -6,6 +6,7 @@
 #endif
 
 #ifndef __dead
+#error "__dead is not defined"
 #define __dead
 #endif

[ghost]

No ... seems that __dead is only defined on some *BSD-Systems (not on Linux and FreeBSD).

[mptre]

This commit 7f5694a might have been a bad idea after all then...

[mptre]

Please give #204 another try.

[ghost]

Still no luck ... the following works here:

--- pick-test.c.orig	2017-07-04 06:53:01.730489953 +0200
+++ pick-test.c	2017-07-04 06:54:00.510488670 +0200
@@ -12,7 +12,7 @@
 #include <string.h>
 #include <unistd.h>
 
-static void	 child(int, int);
+static void	 child(int, int) __attribute__((noreturn));
 static void	 parent(int, int, const char *);
 static char	*parsekeys(const char *);
 static void	 sighandler(int);

[mptre]

My bad, forgot to actually declare the functions as dead. Please give #204 another try.

[ghost]

For me the fallthrough branch is working now.

[ghost]

Confirmed for working with gcc (Debian 7.1.0-7) 7.1.0. Thanks @mptre!
Just curious: I understand that this saves a few bytes in the binary - so just inserting a break (or exit, abort, assert) was no valid option?

[mptre]

Just curious: I understand that this saves a few bytes in the binary - so just inserting a break (or exit, abort, assert) was no valid option?

Your proposed solutions are sure valid options in order to silence the
compiler. However, I don't want to add a code path that cannot be
reached (i.e. dead code). In this particular scenario the compiler can't
deduce that child() will never return so I think we're better of
instructing the compiler about the behavior and intent of the code
rather than lying about it. Hope that makes sense.

[mptre]

Thanks to both of you for compiling pick using modern compilers! The fix
has been merged.