[PATCH] tests: Add break to avoid implicit-fallthrough warning

[PATCH] tests: Add break to avoid implicit-fallthrough warning

[Mark Wielaard]

For some reason gcc might fail to recognize the assert (0) will never
return and emit an implicit-fallthrough warning. Just add a break to
silence it.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 tests/ChangeLog        | 4 ++++
 tests/backtrace-data.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index 70100de4..fb881e76 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2019-12-06  Mark Wielaard  <mark@klomp.org>
+
+	* backtrace-data.c (main): Add break after assert.
+
 2019-12-05  Mark Wielaard  <mark@klomp.org>
 
 	* run-elfclassify.sh: Run elfcompress under testrun.
diff --git a/tests/backtrace-data.c b/tests/backtrace-data.c
index 907b4780..c81880d9 100644
--- a/tests/backtrace-data.c
+++ b/tests/backtrace-data.c
@@ -280,6 +280,7 @@ main (int argc __attribute__ ((unused)), char **argv __attribute__ ((unused)))
   {
     case -1:
       assert (0);
+      break;
     case 0:;
       long l = ptrace (PTRACE_TRACEME, 0, NULL, NULL);
       assert (l == 0);
-- 
2.20.1

Re: [PATCH] tests: Add break to avoid implicit-fallthrough warning

[Florian Weimer]

* Mark Wielaard:

> For some reason gcc might fail to recognize the assert (0) will never
> return and emit an implicit-fallthrough warning. Just add a break to
> silence it.

Is this with -DNDEBUG?  assert (0) expands to basically nothing in that
case.  I'm not sure if we should change that.  We cannot realistically
emit __builtin_unreachable in that case, I think, because it would make
writing incorrect asserts *really* risky with -DNDEBUG.

Thanks,
Florian

Re: [PATCH] tests: Add break to avoid implicit-fallthrough warning

[Mark Wielaard]

On Fri, 2019-12-06 at 17:21 +0100, Florian Weimer wrote:
> * Mark Wielaard:
> 
> > For some reason gcc might fail to recognize the assert (0) will never
> > return and emit an implicit-fallthrough warning. Just add a break to
> > silence it.
> 
> Is this with -DNDEBUG?  assert (0) expands to basically nothing in that
> case.  I'm not sure if we should change that.  We cannot realistically
> emit __builtin_unreachable in that case, I think, because it would make
> writing incorrect asserts *really* risky with -DNDEBUG.

That might also trigger it. But in this case it seems like gcc simply
doesn't fold away the if (expr) ... even though expr is clearly 0 and
just warns about the whole statement. It is odd, maybe caused by this
setup not using include files in the standard place? It just seemed
simpler to just add a break then fight with it.

Cheers,

Mark