Fix implicit-fallthrough warnings in tst-setjmp.c

Message ID alpine.DEB.2.21.1902142203310.4022@digraph.polyomino.org.uk
State Committed
Headers

Commit Message

Joseph Myers Feb. 14, 2019, 10:03 p.m. UTC
  Building the testsuite with -Wextra (together with
-Wno-cast-function-type -Wno-clobbered -Wno-expansion-to-defined
-Wno-missing-field-initializers -Wno-old-style-declaration
-Wno-shift-negative-value -Wno-sign-compare -Wno-type-limits
-Wno-unused-parameter, which reflect the set of -Wextra warnings for
which glibc itself is not currently clean on x86_64) showed up
implicit-fallthrough warnings in tst-setjmp.c.  Those warnings appear
to be false positives, arising from a function "jump" that calls
longjmp not itself being marked as noreturn; thus, this patch adds the
noreturn marking to that function to fix the warnings.

Tested for x86_64.

2019-02-14  Joseph Myers  <joseph@codesourcery.com>

	* setjmp/tst-setjmp.c (jump): Use __attribute__ ((__noreturn__)).
  

Comments

Paul Eggert Feb. 14, 2019, 10:21 p.m. UTC | #1
On 2/14/19 2:03 PM, Joseph Myers wrote:
> -static void
> +static __attribute__ ((__noreturn__)) void

Shouldn't we use the more-standard _Noreturn for this sort of thing? 
cdefs.h defines _Noreturn for pre-C11 GCCs.
  
Joseph Myers Feb. 14, 2019, 10:23 p.m. UTC | #2
On Thu, 14 Feb 2019, Paul Eggert wrote:

> On 2/14/19 2:03 PM, Joseph Myers wrote:
> > -static void
> > +static __attribute__ ((__noreturn__)) void
> 
> Shouldn't we use the more-standard _Noreturn for this sort of thing? cdefs.h
> defines _Noreturn for pre-C11 GCCs.

The attribute seems much more widely used in glibc .c files at present.
  
Paul Eggert Feb. 14, 2019, 10:27 p.m. UTC | #3
On 2/14/19 2:23 PM, Joseph Myers wrote:
>> Shouldn't we use the more-standard _Noreturn for this sort of thing? cdefs.h
>> defines _Noreturn for pre-C11 GCCs.
> The attribute seems much more widely used in glibc .c files at present.

Sure, but that habit predates C11. Now that we have a standard way to 
say "this function does not return" it's surely better to use the 
standard way, at least for new code and arguably even for old code.
  
Joseph Myers Feb. 14, 2019, 10:36 p.m. UTC | #4
On Thu, 14 Feb 2019, Paul Eggert wrote:

> On 2/14/19 2:23 PM, Joseph Myers wrote:
> > > Shouldn't we use the more-standard _Noreturn for this sort of thing?
> > > cdefs.h
> > > defines _Noreturn for pre-C11 GCCs.
> > The attribute seems much more widely used in glibc .c files at present.
> 
> Sure, but that habit predates C11. Now that we have a standard way to say
> "this function does not return" it's surely better to use the standard way, at
> least for new code and arguably even for old code.

The consistent, universal style in glibc, for all files not coming from 
external sources, other than a single use of _Noreturn in 
nptl/thrd_exit.c, is to use the attribute.  You can of course propose 
(with corresponding patch) a change to the style used (recalling that for 
installed headers _Noreturn would reduce portability because of prefix 
attributes not working with GCC 2.7), but it seems clear my patch is 
consistent with the current style and _Noreturn would be inconsistent with 
it.
  
Florian Weimer Feb. 15, 2019, 8:05 a.m. UTC | #5
* Joseph Myers:

> diff --git a/setjmp/tst-setjmp.c b/setjmp/tst-setjmp.c
> index e83e896ebe..b377393ecf 100644
> --- a/setjmp/tst-setjmp.c
> +++ b/setjmp/tst-setjmp.c
> @@ -22,7 +22,7 @@
>  static jmp_buf env;
>  static int last_value = -1, lose = 0;
>  
> -static void
> +static __attribute__ ((__noreturn__)) void
>  jump (int val)
>  {
>    longjmp (env, val);

I think this patch is okay to push.
  

Patch

diff --git a/setjmp/tst-setjmp.c b/setjmp/tst-setjmp.c
index e83e896ebe..b377393ecf 100644
--- a/setjmp/tst-setjmp.c
+++ b/setjmp/tst-setjmp.c
@@ -22,7 +22,7 @@ 
 static jmp_buf env;
 static int last_value = -1, lose = 0;
 
-static void
+static __attribute__ ((__noreturn__)) void
 jump (int val)
 {
   longjmp (env, val);