Re: [PATCH] quotearg: fix compilation failure due to FALLTHROUGH misuse

[Bernhard Voelker]

On 05/26/2017 06:27 AM, Jim Meyering wrote:
> From 6c720446ab4f450b86fe61113096d09f64029de4 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Thu, 25 May 2017 21:25:37 -0700
> Subject: [PATCH] quotearg: fix compilation failure due to FALLTHROUGH misuse
> 
> * lib/quotearg.c (quotearg_buffer_restyled): Revert one FALLTHROUGH
> macro back to /* fall through */ comment.  The macro can apply only
> to a following case statement.  Reported by Assaf Gordon.

...

> diff --git a/lib/quotearg.c b/lib/quotearg.c
> index f1bdf31..06172c1 100644
> --- a/lib/quotearg.c
> +++ b/lib/quotearg.c
> @@ -513,7 +513,7 @@ quotearg_buffer_restyled (char *buffer, size_t buffersize,
>            if (quoting_style == shell_always_quoting_style
>                && elide_outer_quotes)
>              goto force_outer_quoting_style;
> -          FALLTHROUGH;
> +          /* fall through */
>          c_escape:
>            if (backslash_escapes)
>              {

Sorry, you lost me:
"c_escape:" /is/ a case statement, so what's the matter here?
The only difference I can see here is that there is a conditional
goto before the FALLTHROUGH macro.  I'm far away from being an
expert in this region, but this looks like a compiler bug to me.
WDYT?

Furthermore, the macro is now an empty statement for older
compilers.  I've no idea what all the static analyzers in the world
are expecting here, but shouldn't the macro more look like the old
version for such tools?

 #ifndef FALLTHROUGH
 # if __GNUC__ < 7
-#  define FALLTHROUGH ((void) 0)
+#  define FALLTHROUGH /* fall through */
 # else
 #  define FALLTHROUGH __attribute__ ((__fallthrough__))
 # endif

(Okay, there'd still be an empty ';' statement afterward.)

Thanks & have a nice day,
Berny